)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"051c4c5b5468607f24b2f0f94427ee1f083c40a4","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     abhishekkekane \u003cabhishek.kekane@nttdata.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-06-17 23:31:43 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Kill rsync/scp processes before deleting instance"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In the resize operation, during copying files from source to"},{"line_number":10,"context_line":"destination compute node scp/rsync processes are not aborted after"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"fa32b979_ac48a588","line":7,"updated":"2015-06-21 07:40:46.000000000","message":"Please prefix this with libvirt:","commit_id":"05e451add83319b667df4709f6d506ab29373efb"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"66631f5061c9d449d2e58a8327e5c24f997bad3c","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     abhishekkekane \u003cabhishek.kekane@nttdata.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-06-17 23:31:43 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Kill rsync/scp processes before deleting instance"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In the resize operation, during copying files from source to"},{"line_number":10,"context_line":"destination compute node scp/rsync processes are not aborted after"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"fa32b979_c215739b","line":7,"in_reply_to":"fa32b979_ac48a588","updated":"2015-06-23 06:47:59.000000000","message":"Done","commit_id":"05e451add83319b667df4709f6d506ab29373efb"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"051c4c5b5468607f24b2f0f94427ee1f083c40a4","unresolved":false,"context_lines":[{"line_number":287,"context_line":"]"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"PID_CACHE \u003d {}"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"def patch_tpool_proxy():"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa32b979_4c0c7942","line":290,"updated":"2015-06-21 07:40:46.000000000","message":"Would it be possible that we make use of the memorycache provided by openstack:\n\nfrom nova.openstack.common import memorycache","commit_id":"05e451add83319b667df4709f6d506ab29373efb"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"66631f5061c9d449d2e58a8327e5c24f997bad3c","unresolved":false,"context_lines":[{"line_number":287,"context_line":"]"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"PID_CACHE \u003d {}"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"def patch_tpool_proxy():"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa32b979_a2794ffe","line":290,"in_reply_to":"fa32b979_4c0c7942","updated":"2015-06-23 06:47:59.000000000","message":"Done","commit_id":"05e451add83319b667df4709f6d506ab29373efb"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"c4f93c91f0d92e361720f68002964feb5515f54f","unresolved":false,"context_lines":[{"line_number":288,"context_line":"]"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":""},{"line_number":291,"context_line":"MC \u003d None"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":""},{"line_number":294,"context_line":"def _get_cache():"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_7374789b","line":291,"range":{"start_line":291,"start_character":0,"end_line":291,"end_character":2},"updated":"2015-06-23 09:01:34.000000000","message":"I\u0027d prefer this was something more descriptive -- how about PROCESS_ID_CACHE?","commit_id":"c6d5cddc58c54904798b03d6701dac2a5c520f1b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0c062c9c3acdd90561255bc11f12646e9e246b24","unresolved":false,"context_lines":[{"line_number":288,"context_line":"]"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":""},{"line_number":291,"context_line":"MC \u003d None"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":""},{"line_number":294,"context_line":"def _get_cache():"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_193d0dbb","line":291,"in_reply_to":"fa32b979_7374789b","updated":"2015-06-23 09:29:53.000000000","message":"Done","commit_id":"c6d5cddc58c54904798b03d6701dac2a5c520f1b"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"d889c84894e56a9ab42670c5c49f1008d9bf3b41","unresolved":false,"context_lines":[{"line_number":295,"context_line":"    global PROCESS_ID_CACHE"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    if PROCESS_ID_CACHE is None:"},{"line_number":298,"context_line":"        PROCESS_ID_CACHE \u003d memorycache.get_client()"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    return PROCESS_ID_CACHE"},{"line_number":301,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_0b76c96f","line":298,"updated":"2015-07-02 13:14:42.000000000","message":"Using memorycache module doesn\u0027t really seem to be buying us anything over just using a dict(). The memorycache module is written to allow existing code which uses memcached to be converted to an in-process cache. We don\u0027t need that ability, so its simpler to just use a dict() IMHO.","commit_id":"86cd3244584699006c5217dbe1195f0c7a83968b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"5f93a8b06f5109c198896f0cef407dd4d58edd6b","unresolved":false,"context_lines":[{"line_number":295,"context_line":"    global PROCESS_ID_CACHE"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    if PROCESS_ID_CACHE is None:"},{"line_number":298,"context_line":"        PROCESS_ID_CACHE \u003d memorycache.get_client()"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    return PROCESS_ID_CACHE"},{"line_number":301,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_d9a17395","line":298,"in_reply_to":"ba3cc151_0b76c96f","updated":"2015-07-03 06:51:42.000000000","message":"Hi Daniel,\n\nI have received comment to use memcache on PS 1. I have memcache because while scp/rsync process is running and compute service goes down then we will lost the pid store in the dict.","commit_id":"86cd3244584699006c5217dbe1195f0c7a83968b"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"ca67d8000f82de76aaf0dbfa624e205180796267","unresolved":false,"context_lines":[{"line_number":295,"context_line":"    global PROCESS_ID_CACHE"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    if PROCESS_ID_CACHE is None:"},{"line_number":298,"context_line":"        PROCESS_ID_CACHE \u003d memorycache.get_client()"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    return PROCESS_ID_CACHE"},{"line_number":301,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_dfb4f082","line":298,"in_reply_to":"ba3cc151_d9a17395","updated":"2015-07-03 09:09:34.000000000","message":"I\u0027m not convinced that\u0027s needed because when Nova dies, the child processes will get a hangup signal which will kill them off too. You\u0027d only need to track them across restarts if you had daemonized them so their parent PID was no longer Nova and they were in a separate session. I guess if other reviewers want this though, it is not the end of the world to do it.","commit_id":"86cd3244584699006c5217dbe1195f0c7a83968b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"c1202d7bec2006184141fcf49f51aa04804ae6ca","unresolved":false,"context_lines":[{"line_number":295,"context_line":"    global PROCESS_ID_CACHE"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    if PROCESS_ID_CACHE is None:"},{"line_number":298,"context_line":"        PROCESS_ID_CACHE \u003d memorycache.get_client()"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    return PROCESS_ID_CACHE"},{"line_number":301,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_29eb0a7a","line":298,"in_reply_to":"ba3cc151_dfb4f082","updated":"2015-07-07 06:01:50.000000000","message":"Done","commit_id":"86cd3244584699006c5217dbe1195f0c7a83968b"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"d889c84894e56a9ab42670c5c49f1008d9bf3b41","unresolved":false,"context_lines":[{"line_number":6335,"context_line":"            pid \u003d cache.get(cache_key)"},{"line_number":6336,"context_line":""},{"line_number":6337,"context_line":"            if pid and pid \u003d\u003d process.pid:"},{"line_number":6338,"context_line":"                cache.delete(cache_key)"},{"line_number":6339,"context_line":""},{"line_number":6340,"context_line":"        ephemerals \u003d driver.block_device_info_get_ephemerals(block_device_info)"},{"line_number":6341,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_2bc94d59","line":6338,"updated":"2015-07-02 13:14:42.000000000","message":"I\u0027m thinking there are other places in Nova libvirt code where we execute potentially long running commands, which we may need to kill when deleting the instance.\n\nSo rather than inlining cache management in this method, I think we\u0027d be better off creating a dedicate object to allow tracking of arbitrary long running processes against instances eg\n\n  class InstanceJobTracker()\n\n     def __init__()\n        self.jobs \u003d collections.defaultdict(list)\n\n      def add_job(instance, pid)\n        self.jobs[instance.uuid].append(pid)\n\n      def remove_job(instance, pid)\n         ...\n\n      def terminate_jobs(instance)\n         for pid in self.jobs[instance.uuid]:\n             kill(pid)\n\n\nputting this in instancejobtracker.py not driver.py which is already far too huge.\n\nWe can create an instance of this object when storing it as a field on the LibvirtDriver object, not a global variable, and use it whenever we need to track long jobs","commit_id":"86cd3244584699006c5217dbe1195f0c7a83968b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"c1202d7bec2006184141fcf49f51aa04804ae6ca","unresolved":false,"context_lines":[{"line_number":6335,"context_line":"            pid \u003d cache.get(cache_key)"},{"line_number":6336,"context_line":""},{"line_number":6337,"context_line":"            if pid and pid \u003d\u003d process.pid:"},{"line_number":6338,"context_line":"                cache.delete(cache_key)"},{"line_number":6339,"context_line":""},{"line_number":6340,"context_line":"        ephemerals \u003d driver.block_device_info_get_ephemerals(block_device_info)"},{"line_number":6341,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_e9fd223c","line":6338,"in_reply_to":"ba3cc151_1f98181b","updated":"2015-07-07 06:01:50.000000000","message":"Done","commit_id":"86cd3244584699006c5217dbe1195f0c7a83968b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"5f93a8b06f5109c198896f0cef407dd4d58edd6b","unresolved":false,"context_lines":[{"line_number":6335,"context_line":"            pid \u003d cache.get(cache_key)"},{"line_number":6336,"context_line":""},{"line_number":6337,"context_line":"            if pid and pid \u003d\u003d process.pid:"},{"line_number":6338,"context_line":"                cache.delete(cache_key)"},{"line_number":6339,"context_line":""},{"line_number":6340,"context_line":"        ephemerals \u003d driver.block_device_info_get_ephemerals(block_device_info)"},{"line_number":6341,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_39e09fcc","line":6338,"in_reply_to":"ba3cc151_2bc94d59","updated":"2015-07-03 06:51:42.000000000","message":"Hi Daniel,\n\nIs it possible to do this refactoring in a separate patch, because we need to analyze other places where long running commands are running.\n\nIf we change this as per your suggestion, will it be backported easily to stable branches (kilo, juno)?","commit_id":"86cd3244584699006c5217dbe1195f0c7a83968b"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"ca67d8000f82de76aaf0dbfa624e205180796267","unresolved":false,"context_lines":[{"line_number":6335,"context_line":"            pid \u003d cache.get(cache_key)"},{"line_number":6336,"context_line":""},{"line_number":6337,"context_line":"            if pid and pid \u003d\u003d process.pid:"},{"line_number":6338,"context_line":"                cache.delete(cache_key)"},{"line_number":6339,"context_line":""},{"line_number":6340,"context_line":"        ephemerals \u003d driver.block_device_info_get_ephemerals(block_device_info)"},{"line_number":6341,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_1f98181b","line":6338,"in_reply_to":"ba3cc151_39e09fcc","updated":"2015-07-03 09:09:34.000000000","message":"I\u0027m not saying we have to find the other places which do long running comands right now. I would however like to see this formal job tracking class introduced straight away rather than inlining yet more functionality into driver.py - as mentioned before, this file is already way too large and we\u0027ve been actively trying to split out its functionality into separate modules.","commit_id":"86cd3244584699006c5217dbe1195f0c7a83968b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"136d24efd678b739043c0946c13c0965b58b73c5","unresolved":false,"context_lines":[{"line_number":5776,"context_line":"            LOG.debug(\"Image %(image_id)s doesn\u0027t exist anymore on \""},{"line_number":5777,"context_line":"                      \"image service, attempting to copy image \""},{"line_number":5778,"context_line":"                      \"from %(host)s\","},{"line_number":5779,"context_line":"                      {\u0027image_id\u0027: image_id, \u0027host\u0027: fallback_from_host})"},{"line_number":5780,"context_line":"            libvirt_utils.copy_image(src\u003dpath, dest\u003dpath,"},{"line_number":5781,"context_line":"                                     host\u003dfallback_from_host,"},{"line_number":5782,"context_line":"                                     receive\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_d7e01799","line":5779,"updated":"2015-07-10 18:39:11.000000000","message":"What about all of the other calls to libvirt_utils.copy_image in here?  Do we not care about those?\n\nSeems we could make this solution more generic by wrapping calls to libvirt_utils.copy_image in the driver (here) and have our on_execute/on_complete callbacks which store/remove the pid from the cache, and then anything calling copy_image will use that and we\u0027re covered more generically.","commit_id":"baa8688c9cac5b52b0335028b6be65c515f8b55c"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"5c3196555b9e3acc31b61ebc7a47f47e1da28223","unresolved":false,"context_lines":[{"line_number":5776,"context_line":"            LOG.debug(\"Image %(image_id)s doesn\u0027t exist anymore on \""},{"line_number":5777,"context_line":"                      \"image service, attempting to copy image \""},{"line_number":5778,"context_line":"                      \"from %(host)s\","},{"line_number":5779,"context_line":"                      {\u0027image_id\u0027: image_id, \u0027host\u0027: fallback_from_host})"},{"line_number":5780,"context_line":"            libvirt_utils.copy_image(src\u003dpath, dest\u003dpath,"},{"line_number":5781,"context_line":"                                     host\u003dfallback_from_host,"},{"line_number":5782,"context_line":"                                     receive\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a41bdd9_f9dce9e5","line":5779,"in_reply_to":"9a41bdd9_4b1d040e","updated":"2015-07-14 10:23:12.000000000","message":"Please fix only what is referred to by the bug. Potentially you can structure the code to make further fixes easier, but not at the expense of the security fix being as minimal as possible. There is a possibility that the live migration usage of this can cause this but that would need to be confirmed.\n\nI cannot emphasize how important it is to do as little as possible when fixing a security issue.","commit_id":"baa8688c9cac5b52b0335028b6be65c515f8b55c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"bd447b755cc46b8bf6ed0b312db2c64e4c135753","unresolved":false,"context_lines":[{"line_number":5776,"context_line":"            LOG.debug(\"Image %(image_id)s doesn\u0027t exist anymore on \""},{"line_number":5777,"context_line":"                      \"image service, attempting to copy image \""},{"line_number":5778,"context_line":"                      \"from %(host)s\","},{"line_number":5779,"context_line":"                      {\u0027image_id\u0027: image_id, \u0027host\u0027: fallback_from_host})"},{"line_number":5780,"context_line":"            libvirt_utils.copy_image(src\u003dpath, dest\u003dpath,"},{"line_number":5781,"context_line":"                                     host\u003dfallback_from_host,"},{"line_number":5782,"context_line":"                                     receive\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a41bdd9_d852cd66","line":5779,"in_reply_to":"9a41bdd9_f9dce9e5","updated":"2015-07-16 06:27:37.000000000","message":"Done","commit_id":"baa8688c9cac5b52b0335028b6be65c515f8b55c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"244dff3e905e12af52713fab95076f87f8af612a","unresolved":false,"context_lines":[{"line_number":5776,"context_line":"            LOG.debug(\"Image %(image_id)s doesn\u0027t exist anymore on \""},{"line_number":5777,"context_line":"                      \"image service, attempting to copy image \""},{"line_number":5778,"context_line":"                      \"from %(host)s\","},{"line_number":5779,"context_line":"                      {\u0027image_id\u0027: image_id, \u0027host\u0027: fallback_from_host})"},{"line_number":5780,"context_line":"            libvirt_utils.copy_image(src\u003dpath, dest\u003dpath,"},{"line_number":5781,"context_line":"                                     host\u003dfallback_from_host,"},{"line_number":5782,"context_line":"                                     receive\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_b2180120","line":5779,"in_reply_to":"ba3cc151_321e3160","updated":"2015-07-10 18:52:46.000000000","message":"Sure, I\u0027m OK with that too given we need to be able to backport the fix to all stable branches, so keeping it as targeted as possible is a good idea.  However, if we need to handle all calls to libvirt_utils.copy_image, then I\u0027m not sure how else we get around it that isn\u0027t equally messy.","commit_id":"baa8688c9cac5b52b0335028b6be65c515f8b55c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"81a3d5a5132544471f5d111628959dae7695258a","unresolved":false,"context_lines":[{"line_number":5776,"context_line":"            LOG.debug(\"Image %(image_id)s doesn\u0027t exist anymore on \""},{"line_number":5777,"context_line":"                      \"image service, attempting to copy image \""},{"line_number":5778,"context_line":"                      \"from %(host)s\","},{"line_number":5779,"context_line":"                      {\u0027image_id\u0027: image_id, \u0027host\u0027: fallback_from_host})"},{"line_number":5780,"context_line":"            libvirt_utils.copy_image(src\u003dpath, dest\u003dpath,"},{"line_number":5781,"context_line":"                                     host\u003dfallback_from_host,"},{"line_number":5782,"context_line":"                                     receive\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a41bdd9_4b1d040e","line":5779,"in_reply_to":"ba3cc151_b2180120","updated":"2015-07-14 06:59:17.000000000","message":"Hi Matt,\n\nIMO fixing the other calls to libvirt_utils.copy_image in other patch will not create any problem specially in backporting case.\n\nI have received comment form Daniel on PS 4 to create a separate file (instancejobtracker.py) and fix only this issue in this patch.\n\nIf you are okay with the changes in PS 4, I will revert it back.\n\nPlease suggest.","commit_id":"baa8688c9cac5b52b0335028b6be65c515f8b55c"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"30fc1ae11db77effae6489444887af6c998263d6","unresolved":false,"context_lines":[{"line_number":5776,"context_line":"            LOG.debug(\"Image %(image_id)s doesn\u0027t exist anymore on \""},{"line_number":5777,"context_line":"                      \"image service, attempting to copy image \""},{"line_number":5778,"context_line":"                      \"from %(host)s\","},{"line_number":5779,"context_line":"                      {\u0027image_id\u0027: image_id, \u0027host\u0027: fallback_from_host})"},{"line_number":5780,"context_line":"            libvirt_utils.copy_image(src\u003dpath, dest\u003dpath,"},{"line_number":5781,"context_line":"                                     host\u003dfallback_from_host,"},{"line_number":5782,"context_line":"                                     receive\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_321e3160","line":5779,"in_reply_to":"ba3cc151_d7e01799","updated":"2015-07-10 18:44:53.000000000","message":"definitely -1 from me on this in this patch.\n\nit\u0027s meant to fix a (not totally benign ?) security issue. it should be kept as bare-bones as possible","commit_id":"baa8688c9cac5b52b0335028b6be65c515f8b55c"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"84642d1135029cb8fbba9aa96bc0556ac5812ae0","unresolved":false,"context_lines":[{"line_number":6410,"context_line":"                        utils.execute(\u0027mv\u0027, tmp_path, img_path)"},{"line_number":6411,"context_line":"                    else:"},{"line_number":6412,"context_line":"                        libvirt_utils.copy_image(tmp_path, img_path, host\u003ddest)"},{"line_number":6413,"context_line":"                        utils.execute(\u0027rm\u0027, \u0027-f\u0027, tmp_path)"},{"line_number":6414,"context_line":""},{"line_number":6415,"context_line":"                else:  # raw or qcow2 with no backing file"},{"line_number":6416,"context_line":"                    libvirt_utils.copy_image(from_path, img_path, host\u003ddest,"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_570f2788","line":6413,"updated":"2015-07-10 18:39:10.000000000","message":"It is needed here too.","commit_id":"baa8688c9cac5b52b0335028b6be65c515f8b55c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"bd447b755cc46b8bf6ed0b312db2c64e4c135753","unresolved":false,"context_lines":[{"line_number":6410,"context_line":"                        utils.execute(\u0027mv\u0027, tmp_path, img_path)"},{"line_number":6411,"context_line":"                    else:"},{"line_number":6412,"context_line":"                        libvirt_utils.copy_image(tmp_path, img_path, host\u003ddest)"},{"line_number":6413,"context_line":"                        utils.execute(\u0027rm\u0027, \u0027-f\u0027, tmp_path)"},{"line_number":6414,"context_line":""},{"line_number":6415,"context_line":"                else:  # raw or qcow2 with no backing file"},{"line_number":6416,"context_line":"                    libvirt_utils.copy_image(from_path, img_path, host\u003ddest,"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a41bdd9_185cb57b","line":6413,"in_reply_to":"ba3cc151_570f2788","updated":"2015-07-16 06:27:37.000000000","message":"Done","commit_id":"baa8688c9cac5b52b0335028b6be65c515f8b55c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"78b79658c62f79930a39393e60d5249dc5e1559e","unresolved":false,"context_lines":[{"line_number":6296,"context_line":""},{"line_number":6297,"context_line":"            :param process: subprocess object"},{"line_number":6298,"context_line":"            \"\"\""},{"line_number":6299,"context_line":"            self.job_tracker.remove_job(instance, process.pid)"},{"line_number":6300,"context_line":""},{"line_number":6301,"context_line":"        ephemerals \u003d driver.block_device_info_get_ephemerals(block_device_info)"},{"line_number":6302,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9a41bdd9_e7d48448","line":6299,"updated":"2015-07-17 12:32:25.000000000","message":"Adding formal docs for things which are inline methods is real overkill. In fact I\u0027d suggest these could just be inline lambda statements when used.","commit_id":"9cd53cfd91abe2798760cb870f02d5fe3652bf9e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"40a6b83cc33a55c7c5eb3b7f42dcb8b4c583601c","unresolved":false,"context_lines":[{"line_number":6296,"context_line":""},{"line_number":6297,"context_line":"            :param process: subprocess object"},{"line_number":6298,"context_line":"            \"\"\""},{"line_number":6299,"context_line":"            self.job_tracker.remove_job(instance, process.pid)"},{"line_number":6300,"context_line":""},{"line_number":6301,"context_line":"        ephemerals \u003d driver.block_device_info_get_ephemerals(block_device_info)"},{"line_number":6302,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3a50d1a3_203f97d7","line":6299,"in_reply_to":"9a41bdd9_e7d48448","updated":"2015-07-21 07:18:36.000000000","message":"Done","commit_id":"9cd53cfd91abe2798760cb870f02d5fe3652bf9e"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a3b7e03a47641d85e05def1308377f08bc500300","unresolved":false,"context_lines":[{"line_number":6774,"context_line":"            LOG.info(_LI(\u0027Deleting instance files %s\u0027), target_del,"},{"line_number":6775,"context_line":"                     instance\u003dinstance)"},{"line_number":6776,"context_line":"            remaining_path \u003d target_del"},{"line_number":6777,"context_line":"            self.job_tracker.terminate_jobs(instance)"},{"line_number":6778,"context_line":"            try:"},{"line_number":6779,"context_line":"                shutil.rmtree(target_del)"},{"line_number":6780,"context_line":"            except OSError as e:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_ef8442b2","line":6777,"updated":"2015-07-26 04:41:58.000000000","message":"what if the method throws an exception? should we not continue? if this is the case the domain will still be around..","commit_id":"9c06b898a7b5c56e9527a92fcb8f7cb6df15e67b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"c921cea5d48b22d1392572b269634e469c626b13","unresolved":false,"context_lines":[{"line_number":6774,"context_line":"            LOG.info(_LI(\u0027Deleting instance files %s\u0027), target_del,"},{"line_number":6775,"context_line":"                     instance\u003dinstance)"},{"line_number":6776,"context_line":"            remaining_path \u003d target_del"},{"line_number":6777,"context_line":"            self.job_tracker.terminate_jobs(instance)"},{"line_number":6778,"context_line":"            try:"},{"line_number":6779,"context_line":"                shutil.rmtree(target_del)"},{"line_number":6780,"context_line":"            except OSError as e:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_f0ffb631","line":6777,"in_reply_to":"3a50d1a3_ef8442b2","updated":"2015-07-29 09:04:29.000000000","message":"Done","commit_id":"9c06b898a7b5c56e9527a92fcb8f7cb6df15e67b"}],"nova/virt/libvirt/instancejobtracker.py":[{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"84642d1135029cb8fbba9aa96bc0556ac5812ae0","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class InstanceJobTracker(object):"},{"line_number":22,"context_line":"    def __init__(self):"},{"line_number":23,"context_line":"        self.jobs \u003d collections.defaultdict(list)"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def add_job(self, instance, pid):"},{"line_number":26,"context_line":"        \"\"\"Appends process_id of instance to cache."}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_37e623b1","line":23,"updated":"2015-07-10 18:39:10.000000000","message":"should probably be a set instead of a list - not a big deal imho.","commit_id":"baa8688c9cac5b52b0335028b6be65c515f8b55c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"bd447b755cc46b8bf6ed0b312db2c64e4c135753","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class InstanceJobTracker(object):"},{"line_number":22,"context_line":"    def __init__(self):"},{"line_number":23,"context_line":"        self.jobs \u003d collections.defaultdict(list)"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def add_job(self, instance, pid):"},{"line_number":26,"context_line":"        \"\"\"Appends process_id of instance to cache."}],"source_content_type":"text/x-python","patch_set":5,"id":"9a41bdd9_3863b943","line":23,"in_reply_to":"ba3cc151_37e623b1","updated":"2015-07-16 06:27:37.000000000","message":"Hi Nikola,\n\nIMO as of now keep it as list because there might be possibility, If there are some asynchronous tasks executed against a given instance, then there will be more than one subprocess running for that instance which could create a problem.","commit_id":"baa8688c9cac5b52b0335028b6be65c515f8b55c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"136d24efd678b739043c0946c13c0965b58b73c5","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        for pid in self.jobs[instance.uuid]:"},{"line_number":63,"context_line":"            try:"},{"line_number":64,"context_line":"                # Check if the process is alive."},{"line_number":65,"context_line":"                os.kill(pid, 0)"},{"line_number":66,"context_line":"            except OSError:"},{"line_number":67,"context_line":"                # Raised if process is not alive, no action required"},{"line_number":68,"context_line":"                # in this case."}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_d7f7b757","line":65,"updated":"2015-07-10 18:39:11.000000000","message":"See notes from dansmith here:\n\nhttps://review.openstack.org/#/c/200621/4/nova/virt/libvirt/driver.py","commit_id":"baa8688c9cac5b52b0335028b6be65c515f8b55c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"136d24efd678b739043c0946c13c0965b58b73c5","unresolved":false,"context_lines":[{"line_number":71,"context_line":"                # Kill the process"},{"line_number":72,"context_line":"                os.kill(pid, signal.SIGKILL)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"            self.remove_job(instance, pid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_97bfef5f","line":74,"updated":"2015-07-10 18:39:11.000000000","message":"See notes from dansmith here about pre-popping before we iterate:\n\nhttps://review.openstack.org/#/c/200621/4/nova/virt/libvirt/driver.py","commit_id":"baa8688c9cac5b52b0335028b6be65c515f8b55c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"bd447b755cc46b8bf6ed0b312db2c64e4c135753","unresolved":false,"context_lines":[{"line_number":71,"context_line":"                # Kill the process"},{"line_number":72,"context_line":"                os.kill(pid, signal.SIGKILL)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"            self.remove_job(instance, pid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a41bdd9_784861cc","line":74,"in_reply_to":"ba3cc151_97bfef5f","updated":"2015-07-16 06:27:37.000000000","message":"I have added assert check for this.","commit_id":"baa8688c9cac5b52b0335028b6be65c515f8b55c"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"ee2bffe3ab74ec55ff67adbbb60b7a8b0f733d05","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":"        # remove instance.uuid if no pid\u0027s remaining"},{"line_number":51,"context_line":"        if not self.jobs[uuid]:"},{"line_number":52,"context_line":"            del self.jobs[uuid]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    def terminate_jobs(self, instance):"},{"line_number":55,"context_line":"        \"\"\"Kills the running processes for given instance."}],"source_content_type":"text/x-python","patch_set":6,"id":"9a41bdd9_a8b524ed","line":52,"updated":"2015-07-16 13:39:38.000000000","message":"This should be self.jobs.pop(uuid, None).  There\u0027s no locking here so trying to delete it is a potential race.","commit_id":"9cd53cfd91abe2798760cb870f02d5fe3652bf9e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"40a6b83cc33a55c7c5eb3b7f42dcb8b4c583601c","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":"        # remove instance.uuid if no pid\u0027s remaining"},{"line_number":51,"context_line":"        if not self.jobs[uuid]:"},{"line_number":52,"context_line":"            del self.jobs[uuid]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    def terminate_jobs(self, instance):"},{"line_number":55,"context_line":"        \"\"\"Kills the running processes for given instance."}],"source_content_type":"text/x-python","patch_set":6,"id":"3a50d1a3_e05a0f50","line":52,"in_reply_to":"3a50d1a3_c37e0fe4","updated":"2015-07-21 07:18:36.000000000","message":"Done","commit_id":"9cd53cfd91abe2798760cb870f02d5fe3652bf9e"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"945ef67be6fbf779dcaac0b128f540076e7f439f","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":"        # remove instance.uuid if no pid\u0027s remaining"},{"line_number":51,"context_line":"        if not self.jobs[uuid]:"},{"line_number":52,"context_line":"            del self.jobs[uuid]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    def terminate_jobs(self, instance):"},{"line_number":55,"context_line":"        \"\"\"Kills the running processes for given instance."}],"source_content_type":"text/x-python","patch_set":6,"id":"3a50d1a3_c37e0fe4","line":52,"in_reply_to":"9a41bdd9_358624ba","updated":"2015-07-20 20:58:00.000000000","message":"Despite it not being a problem in your testing I would still prefer to see this be an operation that won\u0027t raise an exception if the element does go missing.  We don\u0027t care if the element is missing when it\u0027s removed so there\u0027s no need to raise or handle an exception.","commit_id":"9cd53cfd91abe2798760cb870f02d5fe3652bf9e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"05afca1789a69694b4cbe54dea73049dab8fc93b","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":"        # remove instance.uuid if no pid\u0027s remaining"},{"line_number":51,"context_line":"        if not self.jobs[uuid]:"},{"line_number":52,"context_line":"            del self.jobs[uuid]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    def terminate_jobs(self, instance):"},{"line_number":55,"context_line":"        \"\"\"Kills the running processes for given instance."}],"source_content_type":"text/x-python","patch_set":6,"id":"9a41bdd9_358624ba","line":52,"in_reply_to":"9a41bdd9_a8b524ed","updated":"2015-07-17 07:42:27.000000000","message":"Hi Andrew,\n\nI have tried running this instancejobtracker.py using multi thread about two hours. In one thread I am adding the jobs and in another thread I am removing the jobs. I haven\u0027t encountered any issue so far.\n\nPlease let me know your opinion on the same.","commit_id":"9cd53cfd91abe2798760cb870f02d5fe3652bf9e"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"78b79658c62f79930a39393e60d5249dc5e1559e","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        for pid in pids_to_remove:"},{"line_number":64,"context_line":"            try:"},{"line_number":65,"context_line":"                # Check if the process is alive."},{"line_number":66,"context_line":"                os.kill(pid, 0)"},{"line_number":67,"context_line":"            except OSError:"},{"line_number":68,"context_line":"                # Raised if process is not alive, no action required"},{"line_number":69,"context_line":"                # in this case."}],"source_content_type":"text/x-python","patch_set":6,"id":"9a41bdd9_871fb85a","line":66,"updated":"2015-07-17 12:32:25.000000000","message":"This is racy -- you must catch an error from when you send SIGKILL, by looking for OSError with errno value of ESRCH","commit_id":"9cd53cfd91abe2798760cb870f02d5fe3652bf9e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"40a6b83cc33a55c7c5eb3b7f42dcb8b4c583601c","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        for pid in pids_to_remove:"},{"line_number":64,"context_line":"            try:"},{"line_number":65,"context_line":"                # Check if the process is alive."},{"line_number":66,"context_line":"                os.kill(pid, 0)"},{"line_number":67,"context_line":"            except OSError:"},{"line_number":68,"context_line":"                # Raised if process is not alive, no action required"},{"line_number":69,"context_line":"                # in this case."}],"source_content_type":"text/x-python","patch_set":6,"id":"3a50d1a3_60359ff5","line":66,"in_reply_to":"9a41bdd9_4ba44db8","updated":"2015-07-21 07:18:36.000000000","message":"Done","commit_id":"9cd53cfd91abe2798760cb870f02d5fe3652bf9e"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"6924b985dcc807cf7ccd48515e63fdc5c2d3e6a9","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        for pid in pids_to_remove:"},{"line_number":64,"context_line":"            try:"},{"line_number":65,"context_line":"                # Check if the process is alive."},{"line_number":66,"context_line":"                os.kill(pid, 0)"},{"line_number":67,"context_line":"            except OSError:"},{"line_number":68,"context_line":"                # Raised if process is not alive, no action required"},{"line_number":69,"context_line":"                # in this case."}],"source_content_type":"text/x-python","patch_set":6,"id":"9a41bdd9_4ba44db8","line":66,"in_reply_to":"9a41bdd9_871fb85a","updated":"2015-07-17 16:22:41.000000000","message":"Also see comments on my (now abandoned) patch and how this logic was reworked there https://review.openstack.org/#/c/200621/5/nova/virt/libvirt/driver.py\n\nBasically we want to at least log a warning if the process was in non-interuptable state","commit_id":"9cd53cfd91abe2798760cb870f02d5fe3652bf9e"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"f7216428cb77155aec56808c2098118973f04f3f","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            except OSError:"},{"line_number":75,"context_line":"                # Raised if process is not alive, no action required"},{"line_number":76,"context_line":"                # in this case."},{"line_number":77,"context_line":"                pass"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"            try:"},{"line_number":80,"context_line":"                # Check if the process is still alive."}],"source_content_type":"text/x-python","patch_set":7,"id":"3a50d1a3_6f734b81","line":77,"updated":"2015-07-21 08:17:37.000000000","message":"As mentioned before you should be only ignoring errno \u003d\u003d ESRCH, not ignoring all errors.","commit_id":"cebcd3e914276b54a900a998c4aec8c8951653a9"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"71bc4e94fbd16d7b9e236a30df1dcf7d5792bbde","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            except OSError:"},{"line_number":75,"context_line":"                # Raised if process is not alive, no action required"},{"line_number":76,"context_line":"                # in this case."},{"line_number":77,"context_line":"                pass"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"            try:"},{"line_number":80,"context_line":"                # Check if the process is still alive."}],"source_content_type":"text/x-python","patch_set":7,"id":"3a50d1a3_95358447","line":77,"in_reply_to":"3a50d1a3_6f734b81","updated":"2015-07-21 09:05:06.000000000","message":"Done","commit_id":"cebcd3e914276b54a900a998c4aec8c8951653a9"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"f7216428cb77155aec56808c2098118973f04f3f","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                os.kill(pid, 0)"},{"line_number":82,"context_line":"            except OSError:"},{"line_number":83,"context_line":"                # The process is gone - that\u0027s what we wanted"},{"line_number":84,"context_line":"                pass"},{"line_number":85,"context_line":"            else:"},{"line_number":86,"context_line":"                # The process is still around"},{"line_number":87,"context_line":"                LOG.warn(_LW(\"Failed to kill a long running process \""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a50d1a3_4f68cf87","line":84,"updated":"2015-07-21 08:17:37.000000000","message":"Again, only ignore errno \u003d\u003d ESRCH","commit_id":"cebcd3e914276b54a900a998c4aec8c8951653a9"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"71bc4e94fbd16d7b9e236a30df1dcf7d5792bbde","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                os.kill(pid, 0)"},{"line_number":82,"context_line":"            except OSError:"},{"line_number":83,"context_line":"                # The process is gone - that\u0027s what we wanted"},{"line_number":84,"context_line":"                pass"},{"line_number":85,"context_line":"            else:"},{"line_number":86,"context_line":"                # The process is still around"},{"line_number":87,"context_line":"                LOG.warn(_LW(\"Failed to kill a long running process \""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a50d1a3_950ca495","line":84,"in_reply_to":"3a50d1a3_4f68cf87","updated":"2015-07-21 09:05:06.000000000","message":"Done","commit_id":"cebcd3e914276b54a900a998c4aec8c8951653a9"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"2d9240ef100d971e9a41db82cb6f90b4b46086aa","unresolved":false,"context_lines":[{"line_number":74,"context_line":"                os.kill(pid, signal.SIGKILL)"},{"line_number":75,"context_line":"            except OSError as exc:"},{"line_number":76,"context_line":"                if exc.errno !\u003d errno.ESRCH:"},{"line_number":77,"context_line":"                    raise"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"            try:"},{"line_number":80,"context_line":"                # Check if the process is still alive."}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_74a2f3fd","line":77,"updated":"2015-07-28 09:59:39.000000000","message":"yeah we should not really be raising in this method - agree with Gary.","commit_id":"9c06b898a7b5c56e9527a92fcb8f7cb6df15e67b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"77e858f9d889eb7de5bdeb8170b01e9d2ec7fadf","unresolved":false,"context_lines":[{"line_number":74,"context_line":"                os.kill(pid, signal.SIGKILL)"},{"line_number":75,"context_line":"            except OSError as exc:"},{"line_number":76,"context_line":"                if exc.errno !\u003d errno.ESRCH:"},{"line_number":77,"context_line":"                    raise"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"            try:"},{"line_number":80,"context_line":"                # Check if the process is still alive."}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_b4d4fb1f","line":77,"in_reply_to":"3a50d1a3_74a2f3fd","updated":"2015-07-28 10:04:30.000000000","message":"Hi Nikola,\n\nDaniel has suggested not to ignore all errors.\nPlease refer his comment https://review.openstack.org/#/c/192986/7/nova/virt/libvirt/instancejobtracker.py","commit_id":"9c06b898a7b5c56e9527a92fcb8f7cb6df15e67b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"c921cea5d48b22d1392572b269634e469c626b13","unresolved":false,"context_lines":[{"line_number":74,"context_line":"                os.kill(pid, signal.SIGKILL)"},{"line_number":75,"context_line":"            except OSError as exc:"},{"line_number":76,"context_line":"                if exc.errno !\u003d errno.ESRCH:"},{"line_number":77,"context_line":"                    raise"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"            try:"},{"line_number":80,"context_line":"                # Check if the process is still alive."}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_50f0ea66","line":77,"in_reply_to":"3a50d1a3_b4d4fb1f","updated":"2015-07-29 09:04:29.000000000","message":"Done","commit_id":"9c06b898a7b5c56e9527a92fcb8f7cb6df15e67b"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a3b7e03a47641d85e05def1308377f08bc500300","unresolved":false,"context_lines":[{"line_number":91,"context_line":""},{"line_number":92,"context_line":"            self.remove_job(instance, pid)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        assert not self.jobs.get(instance.uuid)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_cf8946cc","line":94,"updated":"2015-07-26 04:41:58.000000000","message":"do we need this?","commit_id":"9c06b898a7b5c56e9527a92fcb8f7cb6df15e67b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"c921cea5d48b22d1392572b269634e469c626b13","unresolved":false,"context_lines":[{"line_number":91,"context_line":""},{"line_number":92,"context_line":"            self.remove_job(instance, pid)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        assert not self.jobs.get(instance.uuid)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_b0de0ed8","line":94,"in_reply_to":"3a50d1a3_142fef88","updated":"2015-07-29 09:04:29.000000000","message":"Done","commit_id":"9c06b898a7b5c56e9527a92fcb8f7cb6df15e67b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"9f50f1ecc72d717cf9064b28c69e2e6c754100b9","unresolved":false,"context_lines":[{"line_number":91,"context_line":""},{"line_number":92,"context_line":"            self.remove_job(instance, pid)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        assert not self.jobs.get(instance.uuid)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_d604fa4a","line":94,"in_reply_to":"3a50d1a3_142fef88","updated":"2015-07-29 06:08:23.000000000","message":"Hi Nikola,\n\nI have added this because Dan Smith [1] \u0026 Matt [2] has suggested to check this.\nPlease refer to his comment given on your patch,\n\n[1] https://review.openstack.org/#/c/200621/4/nova/virt/libvirt/driver.py\n\n\n[2] https://review.openstack.org/#/c/192986/5/nova/virt/libvirt/instancejobtracker.py\n\nI am going to upload new PS, should I remove it in the same?\n\nPlease suggest.","commit_id":"9c06b898a7b5c56e9527a92fcb8f7cb6df15e67b"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"2d9240ef100d971e9a41db82cb6f90b4b46086aa","unresolved":false,"context_lines":[{"line_number":91,"context_line":""},{"line_number":92,"context_line":"            self.remove_job(instance, pid)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        assert not self.jobs.get(instance.uuid)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_142fef88","line":94,"in_reply_to":"3a50d1a3_cf8946cc","updated":"2015-07-28 09:59:39.000000000","message":"We don\u0027t really, not sure where this came from. No we can remove it in a subsequent patch though seeing that this is a security fix that has been up for several weeks","commit_id":"9c06b898a7b5c56e9527a92fcb8f7cb6df15e67b"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"97e6532140439d0d44be6ce8337d010f3127aaa4","unresolved":false,"context_lines":[{"line_number":78,"context_line":"                    LOG.error(_LE(\u0027Failed to kill process %(pid)s \u0027"},{"line_number":79,"context_line":"                                  \u0027due to %(reason)s, while deleting the \u0027"},{"line_number":80,"context_line":"                                  \u0027instance.\u0027), {\u0027pid\u0027: pid, \u0027reason\u0027: exc},"},{"line_number":81,"context_line":"                              instance\u003dinstance)"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"            try:"},{"line_number":84,"context_line":"                # Check if the process is still alive."}],"source_content_type":"text/x-python","patch_set":10,"id":"3a50d1a3_73f08d67","line":81,"updated":"2015-07-29 10:28:03.000000000","message":"We might as well just return here - but don\u0027t respin just because of this.","commit_id":"7ab75d5b0b75fc3426323bef19bf436a258b9707"}],"nova/virt/libvirt/utils.py":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"d889c84894e56a9ab42670c5c49f1008d9bf3b41","unresolved":false,"context_lines":[{"line_number":328,"context_line":"                    on_completion\u003don_completion)"},{"line_number":329,"context_line":"        else:"},{"line_number":330,"context_line":"            execute(\u0027rsync\u0027, \u0027--sparse\u0027, \u0027--compress\u0027, src, dest,"},{"line_number":331,"context_line":"                    on_execute\u003don_execute, on_completion\u003don_completion)"},{"line_number":332,"context_line":""},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"def write_to_file(path, contents, umask\u003dNone):"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_2824f3c2","line":331,"updated":"2015-07-02 13:14:42.000000000","message":"I\u0027m looking at the oslo.concurrency execute() impl and it seems there\u0027s no guarantee that on_completion will be called in error scenarios. It does\n\n            if on_execute:\n                on_execute(obj)\n\n            result \u003d obj.communicate(process_input)\n\n            obj.stdin.close()  # pylint: disable\u003dE1101\n            _returncode \u003d obj.returncode  # pylint: disable\u003dE1101\n            LOG.log(loglevel, \u0027CMD \"%s\" returned: %s in %0.3fs\u0027,\n                    sanitized_cmd, _returncode, watch.elapsed())\n\n            if on_completion:\n                on_completion(obj)\n\nwhich ignores the pretty real possibility that obj.communicate() can raise an exception. If that happens, on_completion(obj) will never be reached. Thus  we\u0027ll never remove the PID from our cache, and so when we shutdown the instance, we\u0027ll be sending kill to a pid that either no longer exists (best case) or a pid for a completely unrelated process (worst case).\n\nSo I don\u0027t think that we should be relying on this until oslo is fixed to guarantee that on_completion will *always* be called, even on error.","commit_id":"86cd3244584699006c5217dbe1195f0c7a83968b"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"d08139af294f9702edd2efe2112970c2f665553d","unresolved":false,"context_lines":[{"line_number":328,"context_line":"                    on_completion\u003don_completion)"},{"line_number":329,"context_line":"        else:"},{"line_number":330,"context_line":"            execute(\u0027rsync\u0027, \u0027--sparse\u0027, \u0027--compress\u0027, src, dest,"},{"line_number":331,"context_line":"                    on_execute\u003don_execute, on_completion\u003don_completion)"},{"line_number":332,"context_line":""},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"def write_to_file(path, contents, umask\u003dNone):"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_7c7e396b","line":331,"in_reply_to":"ba3cc151_2824f3c2","updated":"2015-07-02 13:54:11.000000000","message":"Reported:\n\n  https://bugs.launchpad.net/oslo.concurrency/+bug/1470868\n\nFix proposed\n\n  https://review.openstack.org/#/c/197983/","commit_id":"86cd3244584699006c5217dbe1195f0c7a83968b"},{"author":{"_account_id":16839,"name":"Cale Rath","email":"ctrath@us.ibm.com","username":"ctrath"},"change_message_id":"0c311cb92b743077cdb758da244eb3bae2dbcf3e","unresolved":false,"context_lines":[{"line_number":215,"context_line":"            # can fall back to scp, without having run out of space"},{"line_number":216,"context_line":"            # on the destination for example."},{"line_number":217,"context_line":"            execute(\u0027rsync\u0027, \u0027--sparse\u0027, \u0027--compress\u0027, \u0027--dry-run\u0027, src, dest,"},{"line_number":218,"context_line":"                    on_execute\u003don_execute, on_completion\u003don_completion)"},{"line_number":219,"context_line":"        except processutils.ProcessExecutionError:"},{"line_number":220,"context_line":"            execute(\u0027scp\u0027, src, dest, on_execute\u003don_execute,"},{"line_number":221,"context_line":"                    on_completion\u003don_completion)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a50d1a3_38681221","line":218,"updated":"2015-07-31 21:11:07.000000000","message":"Does it make sense to perform the on_execute and on_completion calls here since this is only a dry run to determine if rsync is going to work?","commit_id":"7ab75d5b0b75fc3426323bef19bf436a258b9707"}],"requirements.txt":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a3b7e03a47641d85e05def1308377f08bc500300","unresolved":false,"context_lines":[{"line_number":34,"context_line":"stevedore\u003e\u003d1.5.0 # Apache-2.0"},{"line_number":35,"context_line":"setuptools"},{"line_number":36,"context_line":"websockify\u003c0.7,\u003e\u003d0.6.0"},{"line_number":37,"context_line":"oslo.concurrency\u003e\u003d2.3.0 # Apache-2.0"},{"line_number":38,"context_line":"oslo.config\u003e\u003d1.11.0 # Apache-2.0"},{"line_number":39,"context_line":"oslo.context\u003e\u003d0.2.0 # Apache-2.0"},{"line_number":40,"context_line":"oslo.log\u003e\u003d1.6.0 # Apache-2.0"}],"source_content_type":"text/plain","patch_set":9,"id":"3a50d1a3_aff71a58","line":37,"updated":"2015-07-26 04:41:58.000000000","message":"There is a bot that should do this update. You need to rebase your code and you will get:\nhttps://github.com/openstack/nova/blob/master/requirements.txt#L37","commit_id":"9c06b898a7b5c56e9527a92fcb8f7cb6df15e67b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"c921cea5d48b22d1392572b269634e469c626b13","unresolved":false,"context_lines":[{"line_number":34,"context_line":"stevedore\u003e\u003d1.5.0 # Apache-2.0"},{"line_number":35,"context_line":"setuptools"},{"line_number":36,"context_line":"websockify\u003c0.7,\u003e\u003d0.6.0"},{"line_number":37,"context_line":"oslo.concurrency\u003e\u003d2.3.0 # Apache-2.0"},{"line_number":38,"context_line":"oslo.config\u003e\u003d1.11.0 # Apache-2.0"},{"line_number":39,"context_line":"oslo.context\u003e\u003d0.2.0 # Apache-2.0"},{"line_number":40,"context_line":"oslo.log\u003e\u003d1.6.0 # Apache-2.0"}],"source_content_type":"text/plain","patch_set":9,"id":"3a50d1a3_f0d816f1","line":37,"in_reply_to":"3a50d1a3_aff71a58","updated":"2015-07-29 09:04:29.000000000","message":"Done","commit_id":"9c06b898a7b5c56e9527a92fcb8f7cb6df15e67b"}]}