)]}'
{"nova/utils.py":[{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"b986d96070dc8f479fb13f485c60ff08c2b7aea5","unresolved":false,"context_lines":[{"line_number":208,"context_line":"def trycmd(*args, **kwargs):"},{"line_number":209,"context_line":"    \"\"\"A wrapper around execute() to convert error exit exceptions"},{"line_number":210,"context_line":"       to an error message.  If the \u0027discard_warnings\u0027 parameter"},{"line_number":211,"context_line":"       is set to True, then for succeeding commands, stderr is cleared\"\"\""},{"line_number":212,"context_line":"    discard_warnings \u003d kwargs.pop(\u0027discard_warnings\u0027, False)"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAC3%2F%2F9yw%3D","line":211,"updated":"2011-12-11 10:33:40.000000000","message":"HACKING describes how docstrings should be formatted","commit_id":"b774f610fab26d5be50c9e74d7bf45639583f761"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"b986d96070dc8f479fb13f485c60ff08c2b7aea5","unresolved":false,"context_lines":[{"line_number":219,"context_line":"        LOG.debug(err)"},{"line_number":220,"context_line":"        failed \u003d True"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"    if not failed and discard_warnings and err:"},{"line_number":223,"context_line":"        # Handle commands that output to stderr but otherwise succeed"},{"line_number":224,"context_line":"        LOG.debug(err)"},{"line_number":225,"context_line":"        err \u003d \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAC3%2F%2F9yo%3D","line":222,"updated":"2011-12-11 10:33:40.000000000","message":"failed can be unset if execute() throws an exception other than ProcessExecutionError","commit_id":"b774f610fab26d5be50c9e74d7bf45639583f761"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"76a74e41d18d273147d3fab59719e5f53e565004","unresolved":false,"context_lines":[{"line_number":219,"context_line":"        LOG.debug(err)"},{"line_number":220,"context_line":"        failed \u003d True"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"    if not failed and discard_warnings and err:"},{"line_number":223,"context_line":"        # Handle commands that output to stderr but otherwise succeed"},{"line_number":224,"context_line":"        LOG.debug(err)"},{"line_number":225,"context_line":"        err \u003d \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAC3%2F%2F9yU%3D","line":222,"in_reply_to":"AAAAC3%2F%2F9yo%3D","updated":"2011-12-11 21:20:26.000000000","message":"in which case the line is skipped","commit_id":"b774f610fab26d5be50c9e74d7bf45639583f761"}],"nova/virt/disk.py":[{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"a3f95c5cc3b690bd2cb5991789cbca166b6a4dfc","unresolved":false,"context_lines":[{"line_number":335,"context_line":"        \"\"\"Mount a disk image using the object attributes,"},{"line_number":336,"context_line":"        and first supported means provided by the mixin clases.\"\"\""},{"line_number":337,"context_line":"        if self.mode:"},{"line_number":338,"context_line":"            print \"raise exception.Error(_(\u0027image already mounted\u0027))\""},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"        if not self.mount_dir:"},{"line_number":341,"context_line":"            self.mount_dir \u003d tempfile.mkdtemp()"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAC3%2F%2F%2Fyc%3D","line":338,"updated":"2011-12-01 13:19:10.000000000","message":"Oops, I\u0027ll amend this to just raise, rather than print","commit_id":"34cb25f8ccac1d67b9f72506ad995d5df4e7569b"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"d2c7144de4e91935aec5d5de420a5be3d3558e23","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            if err:"},{"line_number":136,"context_line":"                self.error \u003d _(\u0027Failed to load partition: %s\u0027) % err"},{"line_number":137,"context_line":"                return False"},{"line_number":138,"context_line":"            self.mapped_device \u003d \u0027/dev/mapper/%sp%s\u0027 % (self.device.split(\u0027/\u0027)[-1],"},{"line_number":139,"context_line":"                                                        self.partition)"},{"line_number":140,"context_line":"        else:"},{"line_number":141,"context_line":"            self.mapped_device \u003d self.device"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FZM%3D","line":138,"updated":"2011-12-05 09:26:52.000000000","message":"Once kpartx has succeeded, I\u0027d like to see an exception handler which would delete the partitions again if there was an error anywhere before self.mapped is set to True\n\nAs an example - self.device.split(\u0027/\u0027)[-1] should probably never fail but, in practice, some bug elsewhere is bound to trigger an exception here someday and we\u0027ll leak the mapping","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"cad03d0457f00ef5fedad92ba1847290cd3b3931","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            if err:"},{"line_number":136,"context_line":"                self.error \u003d _(\u0027Failed to load partition: %s\u0027) % err"},{"line_number":137,"context_line":"                return False"},{"line_number":138,"context_line":"            self.mapped_device \u003d \u0027/dev/mapper/%sp%s\u0027 % (self.device.split(\u0027/\u0027)[-1],"},{"line_number":139,"context_line":"                                                        self.partition)"},{"line_number":140,"context_line":"        else:"},{"line_number":141,"context_line":"            self.mapped_device \u003d self.device"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FYQ%3D","line":138,"in_reply_to":"AAAAC3%2F%2F%2FZM%3D","updated":"2011-12-05 13:42:45.000000000","message":"Fair enough. There is enough code before the flag to warrant that.","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"d9b75b0b573e60a3706253936460809b2e627e82","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        self.mounted \u003d False"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"    @staticmethod"},{"line_number":190,"context_line":"    def attempt(*args, **kwargs):"},{"line_number":191,"context_line":"        \"\"\"A wrapper around utils.execute() to convert error exit exceptions"},{"line_number":192,"context_line":"           to an error message and return code. If the \u0027discard_warnings\u0027"},{"line_number":193,"context_line":"           parameter is set to True, then any output to stderr is ignored.\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FQg%3D","line":190,"updated":"2011-12-05 19:20:03.000000000","message":"This seems generally useful, perhaps it should go directly into `utils`?","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"cad03d0457f00ef5fedad92ba1847290cd3b3931","unresolved":false,"context_lines":[{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        if not failed and discard_warnings:"},{"line_number":205,"context_line":"            # Handle commands that output to stderr but otherwise succeed"},{"line_number":206,"context_line":"            err \u003d None"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"        return out, err"},{"line_number":209,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FYM%3D","line":206,"updated":"2011-12-05 13:42:45.000000000","message":"s/None/\u0027 \u0027/ at line 200 and 206, as code may try to search etc.","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"d2c7144de4e91935aec5d5de420a5be3d3558e23","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                self.mode \u003d cls.mode"},{"line_number":216,"context_line":"                return True"},{"line_number":217,"context_line":"            else:"},{"line_number":218,"context_line":"                LOG.info(self.error)"},{"line_number":219,"context_line":"                self.do_umount(cls)"},{"line_number":220,"context_line":"        except:"},{"line_number":221,"context_line":"                self.do_umount(cls)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FZI%3D","line":218,"updated":"2011-12-05 09:26:52.000000000","message":"Hmm, I do think this method should raise an exception if there is an error rather than doing this in the caller:\n\n  if img.mount():\n      ...\n  else:\n      raise exception.Error(img.error)\n\nYeah, you need to handle the fact that each handler will raise an exception in turn - probably best to aggregate all errors into the one exception","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"cad03d0457f00ef5fedad92ba1847290cd3b3931","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                self.mode \u003d cls.mode"},{"line_number":216,"context_line":"                return True"},{"line_number":217,"context_line":"            else:"},{"line_number":218,"context_line":"                LOG.info(self.error)"},{"line_number":219,"context_line":"                self.do_umount(cls)"},{"line_number":220,"context_line":"        except:"},{"line_number":221,"context_line":"                self.do_umount(cls)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FYI%3D","line":218,"in_reply_to":"AAAAC3%2F%2F%2FZI%3D","updated":"2011-12-05 13:42:45.000000000","message":"Yes, I\u0027ll aggregate the errors.\nAs for exceptions V normal flow control, I was thinking of only having exceptions for unconsidered errors. The final raise in inject_data() was done, as this was the existing interface. Considering we might be processing arbitrary images here, I thought it best to filter exceptions where possible.\nI\u0027ll think more about it","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"d2c7144de4e91935aec5d5de420a5be3d3558e23","unresolved":false,"context_lines":[{"line_number":217,"context_line":"            else:"},{"line_number":218,"context_line":"                LOG.info(self.error)"},{"line_number":219,"context_line":"                self.do_umount(cls)"},{"line_number":220,"context_line":"        except:"},{"line_number":221,"context_line":"                self.do_umount(cls)"},{"line_number":222,"context_line":"                # XXX: Sometimes exception is cleared"},{"line_number":223,"context_line":"                # so we can fail to reraise here?"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FZA%3D","line":220,"updated":"2011-12-05 09:26:52.000000000","message":"See HACKING:\n\n  Do not write \"except:\", use \"except Exception:\" at the very least","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"d2c7144de4e91935aec5d5de420a5be3d3558e23","unresolved":false,"context_lines":[{"line_number":219,"context_line":"                self.do_umount(cls)"},{"line_number":220,"context_line":"        except:"},{"line_number":221,"context_line":"                self.do_umount(cls)"},{"line_number":222,"context_line":"                # XXX: Sometimes exception is cleared"},{"line_number":223,"context_line":"                # so we can fail to reraise here?"},{"line_number":224,"context_line":"                # That\u0027s because of greenthread call in utils.execute() ?"},{"line_number":225,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FZE%3D","line":222,"updated":"2011-12-05 09:26:52.000000000","message":"Should be XXX(padraig) or whatever","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"d2c7144de4e91935aec5d5de420a5be3d3558e23","unresolved":false,"context_lines":[{"line_number":221,"context_line":"                self.do_umount(cls)"},{"line_number":222,"context_line":"                # XXX: Sometimes exception is cleared"},{"line_number":223,"context_line":"                # so we can fail to reraise here?"},{"line_number":224,"context_line":"                # That\u0027s because of greenthread call in utils.execute() ?"},{"line_number":225,"context_line":"                raise"},{"line_number":226,"context_line":"        return False"},{"line_number":227,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FY8%3D","line":224,"updated":"2011-12-05 09:26:52.000000000","message":"I\u0027m not sure I follow this comment, so perhaps some more detail is required?","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":100,"name":"Johannes Erdfelt","email":"johannes@erdfelt.com","username":"johannes.erdfelt"},"change_message_id":"49d10c0c9cfec49bc38c053f596fb2f7cb6c784a","unresolved":false,"context_lines":[{"line_number":221,"context_line":"                self.do_umount(cls)"},{"line_number":222,"context_line":"                # XXX: Sometimes exception is cleared"},{"line_number":223,"context_line":"                # so we can fail to reraise here?"},{"line_number":224,"context_line":"                # That\u0027s because of greenthread call in utils.execute() ?"},{"line_number":225,"context_line":"                raise"},{"line_number":226,"context_line":"        return False"},{"line_number":227,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FSY%3D","line":224,"in_reply_to":"AAAAC3%2F%2F%2FY8%3D","updated":"2011-12-05 18:33:09.000000000","message":"eventlet can clear the exception context when switching green threads. The same can happen if an exception is raised and caught while inside another exception handler. We have a context manager in nova.utils called save_and_reraise_exception() which handles these cases.\n\nAt least that\u0027s what I\u0027m guessing is being meant :)","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"d2c7144de4e91935aec5d5de420a5be3d3558e23","unresolved":false,"context_lines":[{"line_number":286,"context_line":"        return device"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"    def _free_nbd(self, device):"},{"line_number":289,"context_line":"        self._DEVICES.append(device)"},{"line_number":290,"context_line":""},{"line_number":291,"context_line":"    def get_dev(self):"},{"line_number":292,"context_line":"        device \u003d self._allocate_nbd()"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FY4%3D","line":289,"updated":"2011-12-05 09:26:52.000000000","message":"I see this is just some existing code re-factored, but it scares me a little - if we ever don\u0027t call _free_nbd() for some reason (e.g. broken error handling code), then we could easily end up with an empty DEVICES list here while we still have free nbd devices on the system\n\nDon\u0027t worry about fixing it for this patch, though.","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"d2c7144de4e91935aec5d5de420a5be3d3558e23","unresolved":false,"context_lines":[{"line_number":308,"context_line":"            time.sleep(1)"},{"line_number":309,"context_line":"        else:"},{"line_number":310,"context_line":"            self.error \u003d _(\u0027nbd device %s did not show up\u0027) % device"},{"line_number":311,"context_line":"            self._free_nbd(self.device)"},{"line_number":312,"context_line":"            return False"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"        self.linked \u003d True"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FY0%3D","line":311,"updated":"2011-12-05 09:26:52.000000000","message":"See, it\u0027s places like this I get nervous - we\u0027re absolutely sure nowhere above could raise an exception? If one is raised, we\u0027ve leaked an entry from DEVICES","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"cad03d0457f00ef5fedad92ba1847290cd3b3931","unresolved":false,"context_lines":[{"line_number":308,"context_line":"            time.sleep(1)"},{"line_number":309,"context_line":"        else:"},{"line_number":310,"context_line":"            self.error \u003d _(\u0027nbd device %s did not show up\u0027) % device"},{"line_number":311,"context_line":"            self._free_nbd(self.device)"},{"line_number":312,"context_line":"            return False"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"        self.linked \u003d True"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FYU%3D","line":311,"in_reply_to":"AAAAC3%2F%2F%2FY0%3D","updated":"2011-12-05 13:42:45.000000000","message":"Yep I was thinking of cleaning up all this nbd stuff as per the 3 enumerated points above. For now I was just refactoring.","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"d9b75b0b573e60a3706253936460809b2e627e82","unresolved":false,"context_lines":[{"line_number":323,"context_line":"        self.device \u003d None"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"class _DiskImage(_loopMnt, _nbdMnt):"},{"line_number":327,"context_line":"    \"\"\"Provide operations on a disk image file.\"\"\""},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"    def __init__(self, image, partition\u003dNone, use_cow\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FQU%3D","line":326,"updated":"2011-12-05 19:20:03.000000000","message":"I\u0027m not sure the Mixin pattern works well here since it requires us to mix every driver into the base class.\n\nSeems like a better option would be the Plugin pattern used in other places around nova, including the virt-layer and notifications.\n\nBasically, define an interface with DiskImage in /nova/virt/disk/base.py and then define the implementations via inheritance in /nova/virt/disk/nbd.py, etc.","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"d2c7144de4e91935aec5d5de420a5be3d3558e23","unresolved":false,"context_lines":[{"line_number":337,"context_line":"        self.mode \u003d None"},{"line_number":338,"context_line":"        self.linked \u003d self.mapped \u003d self.mounted \u003d self.mkdir \u003d False"},{"line_number":339,"context_line":"        self.device \u003d self.mapped_device \u003d None"},{"line_number":340,"context_line":"        self.error \u003d \"\""},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        # As a performance tweak, don\u0027t bother trying to"},{"line_number":343,"context_line":"        # directly loopback mount a cow image."}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FYw%3D","line":340,"updated":"2011-12-05 09:26:52.000000000","message":"So, this is initializing the mixin classes ... except \"self.mode \u003d None\"? See, that\u0027s very twisty altogether","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"cad03d0457f00ef5fedad92ba1847290cd3b3931","unresolved":false,"context_lines":[{"line_number":337,"context_line":"        self.mode \u003d None"},{"line_number":338,"context_line":"        self.linked \u003d self.mapped \u003d self.mounted \u003d self.mkdir \u003d False"},{"line_number":339,"context_line":"        self.device \u003d self.mapped_device \u003d None"},{"line_number":340,"context_line":"        self.error \u003d \"\""},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        # As a performance tweak, don\u0027t bother trying to"},{"line_number":343,"context_line":"        # directly loopback mount a cow image."}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FYY%3D","line":340,"in_reply_to":"AAAAC3%2F%2F%2FYw%3D","updated":"2011-12-05 13:42:45.000000000","message":"I suppose s/mode/mode_used/ is clearer?","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"d2c7144de4e91935aec5d5de420a5be3d3558e23","unresolved":false,"context_lines":[{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    def _handler_class(self, mode):"},{"line_number":349,"context_line":"        \"\"\"Look up the appropriate class to use based on MODE.\"\"\""},{"line_number":350,"context_line":"        for cls in self.__class__.__bases__:"},{"line_number":351,"context_line":"            if cls.mode \u003d\u003d mode:"},{"line_number":352,"context_line":"                return cls"},{"line_number":353,"context_line":"        raise exception.Error(_(\"unknown disk image handler: %s\" % mode))"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FZQ%3D","line":350,"updated":"2011-12-05 09:26:52.000000000","message":"I don\u0027t think I like the use of mixin classes here. Basically you\u0027re doing it to just have a list of handler classes, right?\n\nWhy not just use an explicit list?\n\nAs an example of the confusion - the DiskImage class has a \u0027mode\u0027 instance attribute and loopMnt/nbdMnt both have \u0027mode\u0027 class attributes. If we weren\u0027t using inheritance, it\u0027d be much clearer that they\u0027re essentially unrelated.","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"cad03d0457f00ef5fedad92ba1847290cd3b3931","unresolved":false,"context_lines":[{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    def _handler_class(self, mode):"},{"line_number":349,"context_line":"        \"\"\"Look up the appropriate class to use based on MODE.\"\"\""},{"line_number":350,"context_line":"        for cls in self.__class__.__bases__:"},{"line_number":351,"context_line":"            if cls.mode \u003d\u003d mode:"},{"line_number":352,"context_line":"                return cls"},{"line_number":353,"context_line":"        raise exception.Error(_(\"unknown disk image handler: %s\" % mode))"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FYc%3D","line":350,"in_reply_to":"AAAAC3%2F%2F%2FZQ%3D","updated":"2011-12-05 13:42:45.000000000","message":"Ok I\u0027ll see can I split it out. I did it like this as the handler classes read \u0026 write various common DiskImage attributes.","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"d2c7144de4e91935aec5d5de420a5be3d3558e23","unresolved":false,"context_lines":[{"line_number":364,"context_line":""},{"line_number":365,"context_line":"        # This explicit class lookup might be avoided with a multimethod decorator,"},{"line_number":366,"context_line":"        # but this is simple enough not to warrant this abstraction."},{"line_number":367,"context_line":"        return any(self.do_mount(self._handler_class(h)) for h in self.handlers)"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"    def umount(self):"},{"line_number":370,"context_line":"        \"\"\"Unmount a disk image from the file system.\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FYs%3D","line":367,"updated":"2011-12-05 09:26:52.000000000","message":"Since mount_dir isn\u0027t deleted if mount() fails, it looks like the contract here is that the caller should call umount() even if mount() fails? That\u0027s a bit awkward","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"cad03d0457f00ef5fedad92ba1847290cd3b3931","unresolved":false,"context_lines":[{"line_number":364,"context_line":""},{"line_number":365,"context_line":"        # This explicit class lookup might be avoided with a multimethod decorator,"},{"line_number":366,"context_line":"        # but this is simple enough not to warrant this abstraction."},{"line_number":367,"context_line":"        return any(self.do_mount(self._handler_class(h)) for h in self.handlers)"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"    def umount(self):"},{"line_number":370,"context_line":"        \"\"\"Unmount a disk image from the file system.\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAC3%2F%2F%2FYg%3D","line":367,"in_reply_to":"AAAAC3%2F%2F%2FYs%3D","updated":"2011-12-05 13:42:45.000000000","message":"Yes, mount may partially succeed (kpartx etc.), so the umount is required to take down everything. I was thinking about using __enter__ and __exit__ so it could be implicitly done using \u0027with\u0027. I\u0027ll have another look","commit_id":"916d8f22eaba89f16812087b3bf8b2a998431aa5"}],"nova/virt/disk/__init__.py":[{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"0e053b45c356bb858a46024b84ec604c33834461","unresolved":false,"context_lines":[{"line_number":16,"context_line":"\"\"\""},{"line_number":17,"context_line":"Utility methods to resize, repartition, and modify disk images."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Includes injection of SSH PGP keys into authorized_keys file."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"\"\"\""},{"line_number":22,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAC3%2F%2F92o%3D","line":19,"updated":"2011-12-11 01:42:26.000000000","message":"This doc string doesn\u0027t feel right as it\u0027s an exact copy of base.py","commit_id":"b774f610fab26d5be50c9e74d7bf45639583f761"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"9aaa98ecd2c7ccf52e36225524590208e1d00a7f","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"\"\"\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"from base import *"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAC3%2F%2F71s%3D","line":23,"updated":"2011-12-19 22:03:38.000000000","message":"I typically like to avoid import *. Can you remove this?","commit_id":"f97c6652cf0b8df64ac76b125dcbff413199f914"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"ce5fa81eb4139704360e05d727028ad9a3679c1f","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"\"\"\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"from base import *"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAC3%2F%2F7xs%3D","line":23,"in_reply_to":"AAAAC3%2F%2F700%3D","updated":"2011-12-20 14:05:20.000000000","message":"I honestly would prefer that. The clientsbase of this module should be limited to Nova itself, so I don\u0027t think it is a big deal to change the interface here. I typically try to only import objects explicitly or not at all at this level (but that\u0027s mainly in public-facing packages). I\u0027m fine with option 3 if that\u0027s what you prefer.","commit_id":"f97c6652cf0b8df64ac76b125dcbff413199f914"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"4e1697486302243ea14ac244e12c314c19cdc293","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"\"\"\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"from base import *"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAC3%2F%2F700%3D","line":23,"in_reply_to":"AAAAC3%2F%2F71s%3D","updated":"2011-12-19 22:46:07.000000000","message":"Well I was trying to keep the same interface.\nI.E. move from nova.virt.disk module to nova.virt.disk package.\n\nI could just replace __init__.py with base.py,\nthough I got the impression with a couple of other uses\nof \"import *\", that this wasn\u0027t the preferred option.\nGenerally a large __init__ is best avoided to minimize\nstuff loaded if ever using just a sub module.\n\nI could explicit import the public interfaces,\nbut that would introduce a maintenance burden.\n\nI could also change all uses of this package from\nnova.virt.disk... to nova.virt.disk.api... ?\n\nI\u0027m leaning towards the last option,\nif you think this change is required.","commit_id":"f97c6652cf0b8df64ac76b125dcbff413199f914"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"9bbe70fdba42bcbd6919dcc5f816d1044fc6fd9d","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"\"\"\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"from base import *"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAC3%2F%2F7v0%3D","line":23,"in_reply_to":"AAAAC3%2F%2F7xs%3D","updated":"2011-12-20 16:15:15.000000000","message":"Actually I can avoid the redundant \u0027api\u0027 part by doing this in the consuming modules:\n\nfrom nova.virt.disk import api as disk\n\nupdate coming momentarily","commit_id":"f97c6652cf0b8df64ac76b125dcbff413199f914"}],"nova/virt/disk/base.py":[{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"0e053b45c356bb858a46024b84ec604c33834461","unresolved":false,"context_lines":[{"line_number":35,"context_line":"from nova import flags"},{"line_number":36,"context_line":"from nova import log as logging"},{"line_number":37,"context_line":"from nova import utils"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"from nova.virt.disk import loop, nbd"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"LOG \u003d logging.getLogger(\u0027nova.compute.disk\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAC3%2F%2F92c%3D","line":38,"updated":"2011-12-11 01:42:26.000000000","message":"Should be no blank line here.","commit_id":"b774f610fab26d5be50c9e74d7bf45639583f761"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"b986d96070dc8f479fb13f485c60ff08c2b7aea5","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            self._mkdir \u003d True"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"        for h in self.handlers:"},{"line_number":154,"context_line":"            Mounter \u003d self._handler_class(h)"},{"line_number":155,"context_line":"            mounter \u003d Mounter(image\u003dself.image,"},{"line_number":156,"context_line":"                              partition\u003dself.partition,"},{"line_number":157,"context_line":"                              disable_auto_fsck\u003dself.disable_auto_fsck,"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAC3%2F%2F9yk%3D","line":154,"updated":"2011-12-11 10:33:40.000000000","message":"nitpick - think mounter_class is a better variable name\n\nI wonder could this just be:\n\n    mounter_class \u003d utils.import_class(\u0027nova.virt.disk.\u0027 + h + \u0027.Mount\u0027)\n\nThat way people can just drop in a new sub-class and use it without modifying the code in this file","commit_id":"b774f610fab26d5be50c9e74d7bf45639583f761"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"76a74e41d18d273147d3fab59719e5f53e565004","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            self._mkdir \u003d True"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"        for h in self.handlers:"},{"line_number":154,"context_line":"            Mounter \u003d self._handler_class(h)"},{"line_number":155,"context_line":"            mounter \u003d Mounter(image\u003dself.image,"},{"line_number":156,"context_line":"                              partition\u003dself.partition,"},{"line_number":157,"context_line":"                              disable_auto_fsck\u003dself.disable_auto_fsck,"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAC3%2F%2F9yQ%3D","line":154,"in_reply_to":"AAAAC3%2F%2F9yk%3D","updated":"2011-12-11 21:20:26.000000000","message":"I used mounter_cls for line length reasons\n\nWell given _handler_class() has to be updated for each type, I don\u0027t think it\u0027s worth the dynamic import path.  Also we don\u0027t need a plugin dir that needs to be updated at run time *for now at least), so it\u0027s probably OK to simply just hardcode the imports and references in _handler_class() ?","commit_id":"b774f610fab26d5be50c9e74d7bf45639583f761"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"b986d96070dc8f479fb13f485c60ff08c2b7aea5","unresolved":false,"context_lines":[{"line_number":163,"context_line":"                LOG.debug(mounter.error)"},{"line_number":164,"context_line":"                self._errors.append(mounter.error)"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        self.umount()  # rmdir"},{"line_number":167,"context_line":"        return False"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def umount(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAC3%2F%2F9yg%3D","line":166,"updated":"2011-12-11 10:33:40.000000000","message":"This should be in an except/finally clause - if you specify an unknown disk handler or some random error happens, the dir gets leaked","commit_id":"b774f610fab26d5be50c9e74d7bf45639583f761"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"76a74e41d18d273147d3fab59719e5f53e565004","unresolved":false,"context_lines":[{"line_number":163,"context_line":"                LOG.debug(mounter.error)"},{"line_number":164,"context_line":"                self._errors.append(mounter.error)"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        self.umount()  # rmdir"},{"line_number":167,"context_line":"        return False"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def umount(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAC3%2F%2F9yM%3D","line":166,"in_reply_to":"AAAAC3%2F%2F9yg%3D","updated":"2011-12-11 21:20:26.000000000","message":"ah good catch. I missed that whilst changing from the mandatory to optional umount()","commit_id":"b774f610fab26d5be50c9e74d7bf45639583f761"}],"nova/virt/disk/loop.py":[{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"0e053b45c356bb858a46024b84ec604c33834461","unresolved":false,"context_lines":[{"line_number":16,"context_line":"\"\"\"Support for mounting images with the loop device\"\"\""},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from nova import utils"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from nova.virt.disk import mount"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAC3%2F%2F92g%3D","line":19,"updated":"2011-12-11 01:42:26.000000000","message":"Should be no blank line here.","commit_id":"b774f610fab26d5be50c9e74d7bf45639583f761"}],"nova/virt/disk/mount.py":[{"author":{"_account_id":100,"name":"Johannes Erdfelt","email":"johannes@erdfelt.com","username":"johannes.erdfelt"},"change_message_id":"914496d18a3a6721ce96e8360d8670e640cecc12","unresolved":false,"context_lines":[{"line_number":138,"context_line":"                # and so we can fail to reraise here."},{"line_number":139,"context_line":"                # That\u0027s because of greenthread call in the finally: block"},{"line_number":140,"context_line":"                # in utils.exexcute(). See if we can avoid that."},{"line_number":141,"context_line":"                # Note save_and_reraise_exception() doesn\u0027t work in finally:"},{"line_number":142,"context_line":"                raise"},{"line_number":143,"context_line":"        return False"},{"line_number":144,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAC3%2F%2F96A%3D","line":141,"updated":"2011-12-09 23:50:41.000000000","message":"This comment seems irrelevant. Of course it won\u0027t work in finally, there may not be an exception.\n\nAlso, this comment is in an except block, where save_and_reraise_exception would work fine. So I\u0027m not sure what this comment is all about.\n\nAn alternative also is to use a context manager and the with statement. You can see the use of vdi_attached_here in nova/virt/xenapi/vm_utils.py for an example.","commit_id":"d28050f47eb6b8f1cdd0b8b796e37cd738e27524"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"1c2c26707f652c5d9efb34a5573f6a26e130ff9b","unresolved":false,"context_lines":[{"line_number":138,"context_line":"                # and so we can fail to reraise here."},{"line_number":139,"context_line":"                # That\u0027s because of greenthread call in the finally: block"},{"line_number":140,"context_line":"                # in utils.exexcute(). See if we can avoid that."},{"line_number":141,"context_line":"                # Note save_and_reraise_exception() doesn\u0027t work in finally:"},{"line_number":142,"context_line":"                raise"},{"line_number":143,"context_line":"        return False"},{"line_number":144,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAC3%2F%2F95c%3D","line":141,"in_reply_to":"AAAAC3%2F%2F95k%3D","updated":"2011-12-10 00:20:56.000000000","message":"Ok I\u0027ll log a bug. I was intending to look at the issue soon myself. Do I need to resubmit, with this comment removed?","commit_id":"d28050f47eb6b8f1cdd0b8b796e37cd738e27524"},{"author":{"_account_id":100,"name":"Johannes Erdfelt","email":"johannes@erdfelt.com","username":"johannes.erdfelt"},"change_message_id":"c683871a0e1b58a957d77c56b33b8cf45a2662a6","unresolved":false,"context_lines":[{"line_number":138,"context_line":"                # and so we can fail to reraise here."},{"line_number":139,"context_line":"                # That\u0027s because of greenthread call in the finally: block"},{"line_number":140,"context_line":"                # in utils.exexcute(). See if we can avoid that."},{"line_number":141,"context_line":"                # Note save_and_reraise_exception() doesn\u0027t work in finally:"},{"line_number":142,"context_line":"                raise"},{"line_number":143,"context_line":"        return False"},{"line_number":144,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAC3%2F%2F95k%3D","line":141,"in_reply_to":"AAAAC3%2F%2F95s%3D","updated":"2011-12-10 00:09:15.000000000","message":"Ahh, I see. Could you move this comment (or something similar) to utils.execute so we know that it might clear the exception context. Or maybe open up a bug since that\u0027s fairly bad behavior.\n\nI think leaving the comment here might be confusing.","commit_id":"d28050f47eb6b8f1cdd0b8b796e37cd738e27524"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"3cd874a045e789827922f83e2ed8e60688e905c9","unresolved":false,"context_lines":[{"line_number":138,"context_line":"                # and so we can fail to reraise here."},{"line_number":139,"context_line":"                # That\u0027s because of greenthread call in the finally: block"},{"line_number":140,"context_line":"                # in utils.exexcute(). See if we can avoid that."},{"line_number":141,"context_line":"                # Note save_and_reraise_exception() doesn\u0027t work in finally:"},{"line_number":142,"context_line":"                raise"},{"line_number":143,"context_line":"        return False"},{"line_number":144,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAC3%2F%2F95s%3D","line":141,"in_reply_to":"AAAAC3%2F%2F96A%3D","updated":"2011-12-10 00:02:52.000000000","message":"well this path shouldn\u0027t happen (it did while initially testing), so I wrote the TODO, as something to look into in the future.\n\nThe save_and_reraise_exception comment was meant to say that I don\u0027t think that that function is usable around the greenthread.sleep(0) in utils.py (as that\u0027s in a finally block), and that it\u0027s too late to use it in this exception as the greenthread may have run and cleared it?","commit_id":"d28050f47eb6b8f1cdd0b8b796e37cd738e27524"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"b986d96070dc8f479fb13f485c60ff08c2b7aea5","unresolved":false,"context_lines":[{"line_number":134,"context_line":"                self.do_umount()"},{"line_number":135,"context_line":"        except Exception:"},{"line_number":136,"context_line":"                self.do_umount()"},{"line_number":137,"context_line":"                raise"},{"line_number":138,"context_line":"        return False"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    def do_umount(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAC3%2F%2F9ys%3D","line":137,"updated":"2011-12-11 10:33:40.000000000","message":"Indentation error here\n\nAlso, HACKING asks that people do at least:\n\n  except Exception, ex\n      ...\n      raise ex\n\nHow about\n\n    success \u003d False\n    try:\n        success \u003d self.get_dev() and self.map_dev() and self.mnt_dev()\n    finally:\n        if not sucess:\n            self.do_umount()\n        return success","commit_id":"b774f610fab26d5be50c9e74d7bf45639583f761"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"76a74e41d18d273147d3fab59719e5f53e565004","unresolved":false,"context_lines":[{"line_number":134,"context_line":"                self.do_umount()"},{"line_number":135,"context_line":"        except Exception:"},{"line_number":136,"context_line":"                self.do_umount()"},{"line_number":137,"context_line":"                raise"},{"line_number":138,"context_line":"        return False"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    def do_umount(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAC3%2F%2F9yI%3D","line":137,"in_reply_to":"AAAAC3%2F%2F9ys%3D","updated":"2011-12-11 21:20:26.000000000","message":"Ouch. pylint caught that.\nNow all code passes pylint.\nNote I moved the return outside the finally so exceptions are raised","commit_id":"b774f610fab26d5be50c9e74d7bf45639583f761"}],"nova/virt/disk/nbd.py":[{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"0e053b45c356bb858a46024b84ec604c33834461","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"from nova import flags"},{"line_number":22,"context_line":"from nova import utils"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from nova.virt.disk import mount"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"FLAGS \u003d flags.FLAGS"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAC3%2F%2F92k%3D","line":23,"updated":"2011-12-11 01:42:26.000000000","message":"Should be no blank line here.","commit_id":"b774f610fab26d5be50c9e74d7bf45639583f761"}]}