)]}'
{"nova/volume/san.py":[{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"56912fdadeed6908a6c13c948d9fa43f6133d903","unresolved":false,"context_lines":[{"line_number":247,"context_line":"        return luid"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def delete_volume(self, volume):"},{"line_number":250,"context_line":"        \"\"\"Deletes a volume.\"\"\""},{"line_number":251,"context_line":"        zfs_poolname \u003d self._build_zfs_poolname(volume)"},{"line_number":252,"context_line":"        self._run_ssh(\"pfexec /usr/sbin/zfs destroy %s\" %"},{"line_number":253,"context_line":"                      (zfs_poolname))"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAADH%2F%2F%2FeU%3D","side":"PARENT","line":250,"updated":"2011-12-30 15:12:27.000000000","message":"I would prefer that you leave this docstring as it was. We can actually use this for documentation generation (however trivial this comment may be).","commit_id":"d73a0046440b1c22cfb296fb0370905575739ab2"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"56912fdadeed6908a6c13c948d9fa43f6133d903","unresolved":false,"context_lines":[{"line_number":49,"context_line":"                    \u0027SSH port to use with SAN\u0027)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"#Added by Nilanjan"},{"line_number":52,"context_line":"###################"},{"line_number":53,"context_line":"flags.DEFINE_string(\u0027poolname\u0027, \u0027rpool\u0027, \u0027ZFS pool name\u0027)"},{"line_number":54,"context_line":"###################"},{"line_number":55,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAADH%2F%2F%2Fec%3D","line":52,"updated":"2011-12-30 15:12:27.000000000","message":"This isn\u0027t a very sustainable way to track who edits what parts of the files. Think about if hundreds of developers were doing this... the codebase would be half comments! We can use \u0027git blame\u0027 to determine who added specific lines of code much more efficiently :)\n\nIf you need to make a specific note that others should see (not just to track ownership), you should feel free to use the format \u0027#NOTE(nilanjan): ...\u0027","commit_id":"ab560e91130e347bb1cae4860d6370c15ef6aff4"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"56912fdadeed6908a6c13c948d9fa43f6133d903","unresolved":false,"context_lines":[{"line_number":51,"context_line":"#Added by Nilanjan"},{"line_number":52,"context_line":"###################"},{"line_number":53,"context_line":"flags.DEFINE_string(\u0027poolname\u0027, \u0027rpool\u0027, \u0027ZFS pool name\u0027)"},{"line_number":54,"context_line":"###################"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"class SanISCSIDriver(ISCSIDriver):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAADH%2F%2F%2Fd4%3D","line":54,"updated":"2011-12-30 15:12:27.000000000","message":"Please remove this line.","commit_id":"ab560e91130e347bb1cae4860d6370c15ef6aff4"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"56912fdadeed6908a6c13c948d9fa43f6133d903","unresolved":false,"context_lines":[{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def _build_zfs_poolname(self, volume):"},{"line_number":220,"context_line":"        #TODO(justinsb): rpool should be configurable"},{"line_number":221,"context_line":"        #zfs_poolname \u003d \u0027rpool/%s\u0027 % (volume[\u0027name\u0027])"},{"line_number":222,"context_line":"        zfs_poolname \u003d FLAGS.poolname + \u0027/%s\u0027 % (volume[\u0027name\u0027])"},{"line_number":223,"context_line":"        return zfs_poolname"},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAADH%2F%2F%2FeY%3D","line":221,"updated":"2011-12-30 15:12:27.000000000","message":"It makes more sense to just remove this line rather than to leave it in place.","commit_id":"ab560e91130e347bb1cae4860d6370c15ef6aff4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"654f6cf3686cba07340ae02d785a00142de0fb60","unresolved":false,"context_lines":[{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def _build_zfs_poolname(self, volume):"},{"line_number":220,"context_line":"        #TODO(justinsb): rpool should be configurable"},{"line_number":221,"context_line":"        #zfs_poolname \u003d \u0027rpool/%s\u0027 % (volume[\u0027name\u0027])"},{"line_number":222,"context_line":"        zfs_poolname \u003d FLAGS.poolname + \u0027/%s\u0027 % (volume[\u0027name\u0027])"},{"line_number":223,"context_line":"        return zfs_poolname"},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAADH%2F%2F%2FLo%3D","line":221,"in_reply_to":"AAAADH%2F%2F%2FeY%3D","updated":"2012-01-01 15:02:45.000000000","message":"Also, should remove the justinsb comment since this is now configurable :)","commit_id":"ab560e91130e347bb1cae4860d6370c15ef6aff4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"654f6cf3686cba07340ae02d785a00142de0fb60","unresolved":false,"context_lines":[{"line_number":255,"context_line":"        return luid"},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"    def delete_volume(self, volume):"},{"line_number":258,"context_line":"        #Deletes a volume."},{"line_number":259,"context_line":"        zfs_poolname \u003d self._build_zfs_poolname(volume)"},{"line_number":260,"context_line":"        self._run_ssh(\"pfexec /usr/sbin/zfs destroy %s\" %"},{"line_number":261,"context_line":"                      (zfs_poolname))"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAADH%2F%2F%2FLk%3D","line":258,"updated":"2012-01-01 15:02:45.000000000","message":"Please leave this as a docstring, not a # comment.","commit_id":"ab560e91130e347bb1cae4860d6370c15ef6aff4"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"56912fdadeed6908a6c13c948d9fa43f6133d903","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        return self._do_export(volume, force_create\u003dFalse)"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":"    def create_export(self, context, volume):"},{"line_number":278,"context_line":"        #self._do_export(volume, force_create\u003dFalse)"},{"line_number":279,"context_line":"        return self._do_export(volume, force_create\u003dFalse)"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"    def create_export(self, context, volume):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAADH%2F%2F%2FeQ%3D","line":278,"updated":"2011-12-30 15:12:27.000000000","message":"Please remove this line.","commit_id":"ab560e91130e347bb1cae4860d6370c15ef6aff4"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"56912fdadeed6908a6c13c948d9fa43f6133d903","unresolved":false,"context_lines":[{"line_number":279,"context_line":"        return self._do_export(volume, force_create\u003dFalse)"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"    def create_export(self, context, volume):"},{"line_number":282,"context_line":"        #self._do_export(volume, force_create\u003dTrue)"},{"line_number":283,"context_line":"        return self._do_export(volume, force_create\u003dTrue)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    def _do_export(self, volume, force_create):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAADH%2F%2F%2FeM%3D","line":282,"updated":"2011-12-30 15:12:27.000000000","message":"Please remove this line.","commit_id":"ab560e91130e347bb1cae4860d6370c15ef6aff4"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"56912fdadeed6908a6c13c948d9fa43f6133d903","unresolved":false,"context_lines":[{"line_number":326,"context_line":""},{"line_number":327,"context_line":"    def remove_export(self, context, volume):"},{"line_number":328,"context_line":"        \"\"\"Removes an export for a logical volume.\"\"\""},{"line_number":329,"context_line":"        ###### Added by Nilanjan Roy (nilanjan-r)###"},{"line_number":330,"context_line":"        snapshot_count \u003d self._get_snapshot_child_dataset_count(volume)"},{"line_number":331,"context_line":"        if snapshot_count \u003e 1:"},{"line_number":332,"context_line":"            LOG.debug(\"%s contains snapshot. Cannot delete volume %s\" %"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAADH%2F%2F%2FeA%3D","line":329,"updated":"2011-12-30 15:12:27.000000000","message":"Please remove this line.","commit_id":"ab560e91130e347bb1cae4860d6370c15ef6aff4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"654f6cf3686cba07340ae02d785a00142de0fb60","unresolved":false,"context_lines":[{"line_number":329,"context_line":"        ###### Added by Nilanjan Roy (nilanjan-r)###"},{"line_number":330,"context_line":"        snapshot_count \u003d self._get_snapshot_child_dataset_count(volume)"},{"line_number":331,"context_line":"        if snapshot_count \u003e 1:"},{"line_number":332,"context_line":"            LOG.debug(\"%s contains snapshot. Cannot delete volume %s\" %"},{"line_number":333,"context_line":"                     (volume[\u0027name\u0027], volume[\u0027name\u0027]))"},{"line_number":334,"context_line":"            raise exception.VolumeIsBusy(volume[\u0027name\u0027])"},{"line_number":335,"context_line":"        ############################"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAADH%2F%2F%2FLg%3D","line":332,"updated":"2012-01-01 15:02:45.000000000","message":"Please i18n this line. It has \u003e1 interpolation, so should be done like so:\n\n  volume_name \u003d volume[\u0027name\u0027]\n  msg \u003d _(\"%(volume_name)s contains snapshot. \"\n               \"Cannot delete volume %(volume_name)s\") % locals()\n  LOG.debug(msg)","commit_id":"ab560e91130e347bb1cae4860d6370c15ef6aff4"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"56912fdadeed6908a6c13c948d9fa43f6133d903","unresolved":false,"context_lines":[{"line_number":332,"context_line":"            LOG.debug(\"%s contains snapshot. Cannot delete volume %s\" %"},{"line_number":333,"context_line":"                     (volume[\u0027name\u0027], volume[\u0027name\u0027]))"},{"line_number":334,"context_line":"            raise exception.VolumeIsBusy(volume[\u0027name\u0027])"},{"line_number":335,"context_line":"        ############################"},{"line_number":336,"context_line":"        # This is the reverse of _do_export"},{"line_number":337,"context_line":"        luid \u003d self._get_luid(volume)"},{"line_number":338,"context_line":"        iscsi_name \u003d self._build_iscsi_target_name(volume)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAADH%2F%2F%2Fd8%3D","line":335,"updated":"2011-12-30 15:12:27.000000000","message":"Please remove this line.","commit_id":"ab560e91130e347bb1cae4860d6370c15ef6aff4"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"56912fdadeed6908a6c13c948d9fa43f6133d903","unresolved":false,"context_lines":[{"line_number":358,"context_line":"            self._run_ssh(\"pfexec /usr/sbin/sbdadm delete-lu %s\" %"},{"line_number":359,"context_line":"                          (luid))"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"    ########Added by Nilanjan (nilanjan-r)##################"},{"line_number":362,"context_line":"    def create_snapshot(self, snapshot):"},{"line_number":363,"context_line":"        \"\"\"Create a zfs snapshot\"\"\""},{"line_number":364,"context_line":"        orig_zfs_poolname \u003d FLAGS.poolname + \u0027/%s\u0027 % (snapshot[\u0027volume_name\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAADH%2F%2F%2FeI%3D","line":361,"updated":"2011-12-30 15:12:27.000000000","message":"Please remove this line.","commit_id":"ab560e91130e347bb1cae4860d6370c15ef6aff4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"654f6cf3686cba07340ae02d785a00142de0fb60","unresolved":false,"context_lines":[{"line_number":374,"context_line":"        dest_zfs_poolname \u003d FLAGS.poolname + \u0027/%s\u0027 % (volume[\u0027name\u0027])"},{"line_number":375,"context_line":"        dest_snapshot_name \u003d \"%s@%s\" % (dest_zfs_poolname,"},{"line_number":376,"context_line":"                  self._escape_snapshot(snapshot[\u0027name\u0027]))"},{"line_number":377,"context_line":"        \"\"\"Creates a zfs volume from the snapshot\"\"\""},{"line_number":378,"context_line":"        cmd_1 \u003d (\"pfexec /usr/sbin/zfs send %s | \""},{"line_number":379,"context_line":"                 \"pfexec /usr/sbin/zfs recv %s \" %"},{"line_number":380,"context_line":"                 (orig_snapshot_name, dest_zfs_poolname))"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAADH%2F%2F%2FLc%3D","line":377,"updated":"2012-01-01 15:02:45.000000000","message":"The above should be a # comment, not a docstring.","commit_id":"ab560e91130e347bb1cae4860d6370c15ef6aff4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"654f6cf3686cba07340ae02d785a00142de0fb60","unresolved":false,"context_lines":[{"line_number":384,"context_line":"        pool/new_volume@snap automatically."},{"line_number":385,"context_line":"        We need to delete the snapshot post creation"},{"line_number":386,"context_line":"        of the zfs volume."},{"line_number":387,"context_line":"        \"\"\""},{"line_number":388,"context_line":"        cmd_2 \u003d (\"pfexec  /usr/sbin/zfs destroy %s\" % (dest_snapshot_name))"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"        self._run_ssh(cmd_1)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAADH%2F%2F%2FLY%3D","line":387,"updated":"2012-01-01 15:02:45.000000000","message":"The above should be a # comment, not a docstring","commit_id":"ab560e91130e347bb1cae4860d6370c15ef6aff4"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"56912fdadeed6908a6c13c948d9fa43f6133d903","unresolved":false,"context_lines":[{"line_number":411,"context_line":"               \"grep %s| wc -l\" % (zfs_poolname, zfs_poolname))"},{"line_number":412,"context_line":"        (out, _err) \u003d self._run_ssh(cmd, check_exit_code\u003dFalse)"},{"line_number":413,"context_line":"        return int(out.strip())"},{"line_number":414,"context_line":"    ######################################################"},{"line_number":415,"context_line":""},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"class HpSanISCSIDriver(SanISCSIDriver):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAADH%2F%2F%2FeE%3D","line":414,"updated":"2011-12-30 15:12:27.000000000","message":"Please remove this line.","commit_id":"ab560e91130e347bb1cae4860d6370c15ef6aff4"},{"author":{"_account_id":144,"name":"justinsb","email":"justin@fathomdb.com","username":"justin-fathomdb"},"change_message_id":"a6c19e374f0467b4651d94c0c695ca1c83884788","unresolved":false,"context_lines":[{"line_number":322,"context_line":"        snapshot_count \u003d self._get_snapshot_child_dataset_count(volume)"},{"line_number":323,"context_line":"        if snapshot_count \u003e 1:"},{"line_number":324,"context_line":"            #NOTE(nilanjan):If snapshot is present,"},{"line_number":325,"context_line":"            #leave export as it is and raise exception."},{"line_number":326,"context_line":"            volume_name \u003d volume[\u0027name\u0027]"},{"line_number":327,"context_line":"            msg \u003d _(\"%(volume_name)s contains snapshot. \""},{"line_number":328,"context_line":"                    \"Cannot delete volume %(volume_name)s\") % locals()"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAADH%2F%2F%2B9w%3D","line":325,"updated":"2012-01-03 17:07:44.000000000","message":"Is this restriction something you\u0027re copying from one of the other drivers (i.e. part of the OpenStack contract), or is it something that Solaris/ZFS imposes?  A #note as to why this is needed would be helpful here.","commit_id":"3add4cdb0d783e6ce1326eb3e048eaf29a63e8d7"},{"author":{"_account_id":2256,"name":"Nilanjan Roy","email":"nilanjan1.roy@gmail.com","username":"nilanjan-r"},"change_message_id":"acf01f9816ce1ce484742d61edf444cd8a9923ad","unresolved":false,"context_lines":[{"line_number":322,"context_line":"        snapshot_count \u003d self._get_snapshot_child_dataset_count(volume)"},{"line_number":323,"context_line":"        if snapshot_count \u003e 1:"},{"line_number":324,"context_line":"            #NOTE(nilanjan):If snapshot is present,"},{"line_number":325,"context_line":"            #leave export as it is and raise exception."},{"line_number":326,"context_line":"            volume_name \u003d volume[\u0027name\u0027]"},{"line_number":327,"context_line":"            msg \u003d _(\"%(volume_name)s contains snapshot. \""},{"line_number":328,"context_line":"                    \"Cannot delete volume %(volume_name)s\") % locals()"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAADH%2F%2F%2BuE%3D","line":325,"in_reply_to":"AAAADH%2F%2F%2B9w%3D","updated":"2012-01-05 07:15:05.000000000","message":"Hi Justin,\n\nBoth are true. I have copied this restriction from the LVM based driver. Also in case of ZFS we can\u0027t remove a dataset (volume) which contains a child dataset (snapshot in this case) simply by running \"zfs destroy \u003cvolume_name\u003e\" -- This command is used in method \"delete_volume\". For that we have to run \"zfs destroy -R  \u003cvolume_name\u003e\".","commit_id":"3add4cdb0d783e6ce1326eb3e048eaf29a63e8d7"},{"author":{"_account_id":144,"name":"justinsb","email":"justin@fathomdb.com","username":"justin-fathomdb"},"change_message_id":"a6c19e374f0467b4651d94c0c695ca1c83884788","unresolved":false,"context_lines":[{"line_number":357,"context_line":"        \"\"\"Create a zfs snapshot\"\"\""},{"line_number":358,"context_line":"        orig_zfs_poolname \u003d FLAGS.poolname + \u0027/%s\u0027 % (snapshot[\u0027volume_name\u0027])"},{"line_number":359,"context_line":"        snapshot_name \u003d \"%s@%s\" % (orig_zfs_poolname,"},{"line_number":360,"context_line":"                                   self._escape_snapshot(snapshot[\u0027name\u0027]))"},{"line_number":361,"context_line":"        self._run_ssh(\"pfexec /usr/sbin/zfs snapshot %s\" % (snapshot_name))"},{"line_number":362,"context_line":""},{"line_number":363,"context_line":"    def create_volume_from_snapshot(self, volume, snapshot):"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAADH%2F%2F%2B%2BQ%3D","line":360,"updated":"2012-01-03 17:07:44.000000000","message":"Is _escape_snapshot the right escaping function here?  It looks specific to LVM.","commit_id":"3add4cdb0d783e6ce1326eb3e048eaf29a63e8d7"},{"author":{"_account_id":2256,"name":"Nilanjan Roy","email":"nilanjan1.roy@gmail.com","username":"nilanjan-r"},"change_message_id":"acf01f9816ce1ce484742d61edf444cd8a9923ad","unresolved":false,"context_lines":[{"line_number":357,"context_line":"        \"\"\"Create a zfs snapshot\"\"\""},{"line_number":358,"context_line":"        orig_zfs_poolname \u003d FLAGS.poolname + \u0027/%s\u0027 % (snapshot[\u0027volume_name\u0027])"},{"line_number":359,"context_line":"        snapshot_name \u003d \"%s@%s\" % (orig_zfs_poolname,"},{"line_number":360,"context_line":"                                   self._escape_snapshot(snapshot[\u0027name\u0027]))"},{"line_number":361,"context_line":"        self._run_ssh(\"pfexec /usr/sbin/zfs snapshot %s\" % (snapshot_name))"},{"line_number":362,"context_line":""},{"line_number":363,"context_line":"    def create_volume_from_snapshot(self, volume, snapshot):"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAADH%2F%2F%2Btk%3D","line":360,"in_reply_to":"AAAADH%2F%2F%2B%2BQ%3D","updated":"2012-01-05 07:15:05.000000000","message":"I just followed what has been done for LVM. But we can use without any escaping function as well.","commit_id":"3add4cdb0d783e6ce1326eb3e048eaf29a63e8d7"},{"author":{"_account_id":144,"name":"justinsb","email":"justin@fathomdb.com","username":"justin-fathomdb"},"change_message_id":"a6c19e374f0467b4651d94c0c695ca1c83884788","unresolved":false,"context_lines":[{"line_number":358,"context_line":"        orig_zfs_poolname \u003d FLAGS.poolname + \u0027/%s\u0027 % (snapshot[\u0027volume_name\u0027])"},{"line_number":359,"context_line":"        snapshot_name \u003d \"%s@%s\" % (orig_zfs_poolname,"},{"line_number":360,"context_line":"                                   self._escape_snapshot(snapshot[\u0027name\u0027]))"},{"line_number":361,"context_line":"        self._run_ssh(\"pfexec /usr/sbin/zfs snapshot %s\" % (snapshot_name))"},{"line_number":362,"context_line":""},{"line_number":363,"context_line":"    def create_volume_from_snapshot(self, volume, snapshot):"},{"line_number":364,"context_line":"        \"\"\"Creates a volume from a snapshot\"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAADH%2F%2F%2B%2BM%3D","line":361,"updated":"2012-01-03 17:07:44.000000000","message":"FYI, I have a concurrent patch that changes the command execution functions to take arrays of string arguments; if mine lands first there will definitely be a conflict!  Just a heads-up, as you\u0027ve done the right thing in coding to the code we have today.","commit_id":"3add4cdb0d783e6ce1326eb3e048eaf29a63e8d7"},{"author":{"_account_id":2256,"name":"Nilanjan Roy","email":"nilanjan1.roy@gmail.com","username":"nilanjan-r"},"change_message_id":"acf01f9816ce1ce484742d61edf444cd8a9923ad","unresolved":false,"context_lines":[{"line_number":358,"context_line":"        orig_zfs_poolname \u003d FLAGS.poolname + \u0027/%s\u0027 % (snapshot[\u0027volume_name\u0027])"},{"line_number":359,"context_line":"        snapshot_name \u003d \"%s@%s\" % (orig_zfs_poolname,"},{"line_number":360,"context_line":"                                   self._escape_snapshot(snapshot[\u0027name\u0027]))"},{"line_number":361,"context_line":"        self._run_ssh(\"pfexec /usr/sbin/zfs snapshot %s\" % (snapshot_name))"},{"line_number":362,"context_line":""},{"line_number":363,"context_line":"    def create_volume_from_snapshot(self, volume, snapshot):"},{"line_number":364,"context_line":"        \"\"\"Creates a volume from a snapshot\"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAADH%2F%2F%2BuA%3D","line":361,"in_reply_to":"AAAADH%2F%2F%2B%2BM%3D","updated":"2012-01-05 07:15:05.000000000","message":"Hi Justin,\n\nWhat should I do for this?","commit_id":"3add4cdb0d783e6ce1326eb3e048eaf29a63e8d7"},{"author":{"_account_id":144,"name":"justinsb","email":"justin@fathomdb.com","username":"justin-fathomdb"},"change_message_id":"a6c19e374f0467b4651d94c0c695ca1c83884788","unresolved":false,"context_lines":[{"line_number":364,"context_line":"        \"\"\"Creates a volume from a snapshot\"\"\""},{"line_number":365,"context_line":"        orig_zfs_poolname \u003d FLAGS.poolname + \u0027/%s\u0027 % (snapshot[\u0027volume_name\u0027])"},{"line_number":366,"context_line":"        orig_snapshot_name \u003d \"%s@%s\" % (orig_zfs_poolname,"},{"line_number":367,"context_line":"                  self._escape_snapshot(snapshot[\u0027name\u0027]))"},{"line_number":368,"context_line":"        dest_zfs_poolname \u003d FLAGS.poolname + \u0027/%s\u0027 % (volume[\u0027name\u0027])"},{"line_number":369,"context_line":"        dest_snapshot_name \u003d \"%s@%s\" % (dest_zfs_poolname,"},{"line_number":370,"context_line":"                  self._escape_snapshot(snapshot[\u0027name\u0027]))"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAADH%2F%2F%2B%2BE%3D","line":367,"updated":"2012-01-03 17:07:44.000000000","message":"Maybe extract a function that builds a zfs path (taking the volume  name and snapshot name)?  e.g. You could let _build_zfs_poolname take an optional snapshot argument.","commit_id":"3add4cdb0d783e6ce1326eb3e048eaf29a63e8d7"},{"author":{"_account_id":2256,"name":"Nilanjan Roy","email":"nilanjan1.roy@gmail.com","username":"nilanjan-r"},"change_message_id":"acf01f9816ce1ce484742d61edf444cd8a9923ad","unresolved":false,"context_lines":[{"line_number":364,"context_line":"        \"\"\"Creates a volume from a snapshot\"\"\""},{"line_number":365,"context_line":"        orig_zfs_poolname \u003d FLAGS.poolname + \u0027/%s\u0027 % (snapshot[\u0027volume_name\u0027])"},{"line_number":366,"context_line":"        orig_snapshot_name \u003d \"%s@%s\" % (orig_zfs_poolname,"},{"line_number":367,"context_line":"                  self._escape_snapshot(snapshot[\u0027name\u0027]))"},{"line_number":368,"context_line":"        dest_zfs_poolname \u003d FLAGS.poolname + \u0027/%s\u0027 % (volume[\u0027name\u0027])"},{"line_number":369,"context_line":"        dest_snapshot_name \u003d \"%s@%s\" % (dest_zfs_poolname,"},{"line_number":370,"context_line":"                  self._escape_snapshot(snapshot[\u0027name\u0027]))"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAADH%2F%2F%2BtM%3D","line":367,"in_reply_to":"AAAADH%2F%2F%2B%2BE%3D","updated":"2012-01-05 07:15:05.000000000","message":"I shall be writing the extract function or change the _build_zfs_poolname accordingly and amend the commit.","commit_id":"3add4cdb0d783e6ce1326eb3e048eaf29a63e8d7"},{"author":{"_account_id":144,"name":"justinsb","email":"justin@fathomdb.com","username":"justin-fathomdb"},"change_message_id":"a6c19e374f0467b4651d94c0c695ca1c83884788","unresolved":false,"context_lines":[{"line_number":371,"context_line":"        #Creates a zfs volume from the snapshot"},{"line_number":372,"context_line":"        cmd_1 \u003d (\"pfexec /usr/sbin/zfs send %s | \""},{"line_number":373,"context_line":"                 \"pfexec /usr/sbin/zfs recv %s \" %"},{"line_number":374,"context_line":"                  (orig_snapshot_name, dest_zfs_poolname))"},{"line_number":375,"context_line":"        #zfs send pool/volume@snap | zfs recv pool/new_volume"},{"line_number":376,"context_line":"        #command creates a snapshot"},{"line_number":377,"context_line":"        #pool/new_volume@snap automatically."}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAADH%2F%2F%2B%2BA%3D","line":374,"updated":"2012-01-03 17:07:44.000000000","message":"Couldn\u0027t you use \"zfs clone\" here?  I\u0027m guessing you did this on purpose, so can you add a comment as to why.","commit_id":"3add4cdb0d783e6ce1326eb3e048eaf29a63e8d7"},{"author":{"_account_id":2256,"name":"Nilanjan Roy","email":"nilanjan1.roy@gmail.com","username":"nilanjan-r"},"change_message_id":"acf01f9816ce1ce484742d61edf444cd8a9923ad","unresolved":false,"context_lines":[{"line_number":371,"context_line":"        #Creates a zfs volume from the snapshot"},{"line_number":372,"context_line":"        cmd_1 \u003d (\"pfexec /usr/sbin/zfs send %s | \""},{"line_number":373,"context_line":"                 \"pfexec /usr/sbin/zfs recv %s \" %"},{"line_number":374,"context_line":"                  (orig_snapshot_name, dest_zfs_poolname))"},{"line_number":375,"context_line":"        #zfs send pool/volume@snap | zfs recv pool/new_volume"},{"line_number":376,"context_line":"        #command creates a snapshot"},{"line_number":377,"context_line":"        #pool/new_volume@snap automatically."}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAADH%2F%2F%2BtY%3D","line":374,"in_reply_to":"AAAADH%2F%2F%2B%2BA%3D","updated":"2012-01-05 07:15:05.000000000","message":"If we use \"zfs clone\" then the new volume will be a dependent of the source volume. We can\u0027t delete the source volume unless the new volume is also deleted. To avoid this kind of situation I have used \"zfs send and \"zfs  recv\" to create a new dataset.","commit_id":"3add4cdb0d783e6ce1326eb3e048eaf29a63e8d7"},{"author":{"_account_id":144,"name":"justinsb","email":"justin@fathomdb.com","username":"justin-fathomdb"},"change_message_id":"a6c19e374f0467b4651d94c0c695ca1c83884788","unresolved":false,"context_lines":[{"line_number":387,"context_line":"        else:"},{"line_number":388,"context_line":"            sizestr \u003d \u0027%sG\u0027 % volume[\u0027size\u0027]"},{"line_number":389,"context_line":"            self._run_ssh(\"pfexec /usr/sbin/zfs set volsize\u003d%s %s\" %"},{"line_number":390,"context_line":"                         (sizestr, dest_zfs_poolname))"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    def delete_snapshot(self, snapshot):"},{"line_number":393,"context_line":"        \"\"\"Deletes a zfs snapshot\"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAADH%2F%2F%2B98%3D","line":390,"updated":"2012-01-03 17:07:44.000000000","message":"Cool - but do any of the other drivers support volume resizing here?  Just wondering if this is actually ever used!","commit_id":"3add4cdb0d783e6ce1326eb3e048eaf29a63e8d7"},{"author":{"_account_id":2256,"name":"Nilanjan Roy","email":"nilanjan1.roy@gmail.com","username":"nilanjan-r"},"change_message_id":"acf01f9816ce1ce484742d61edf444cd8a9923ad","unresolved":false,"context_lines":[{"line_number":387,"context_line":"        else:"},{"line_number":388,"context_line":"            sizestr \u003d \u0027%sG\u0027 % volume[\u0027size\u0027]"},{"line_number":389,"context_line":"            self._run_ssh(\"pfexec /usr/sbin/zfs set volsize\u003d%s %s\" %"},{"line_number":390,"context_line":"                         (sizestr, dest_zfs_poolname))"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    def delete_snapshot(self, snapshot):"},{"line_number":393,"context_line":"        \"\"\"Deletes a zfs snapshot\"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAADH%2F%2F%2BtU%3D","line":390,"in_reply_to":"AAAADH%2F%2F%2B98%3D","updated":"2012-01-05 07:15:05.000000000","message":"I don\u0027t think other drivers have used it. I just wanted to add this functionality here.","commit_id":"3add4cdb0d783e6ce1326eb3e048eaf29a63e8d7"},{"author":{"_account_id":144,"name":"justinsb","email":"justin@fathomdb.com","username":"justin-fathomdb"},"change_message_id":"a6c19e374f0467b4651d94c0c695ca1c83884788","unresolved":false,"context_lines":[{"line_number":400,"context_line":"        \"\"\"Checks for presense of snapshot of a volume\"\"\""},{"line_number":401,"context_line":"        zfs_poolname \u003d self._build_zfs_poolname(volume)"},{"line_number":402,"context_line":"        cmd \u003d (\"pfexec /usr/sbin/zfs list -r %s | \""},{"line_number":403,"context_line":"               \"grep %s| wc -l\" % (zfs_poolname, zfs_poolname))"},{"line_number":404,"context_line":"        (out, _err) \u003d self._run_ssh(cmd, check_exit_code\u003dFalse)"},{"line_number":405,"context_line":"        return int(out.strip())"},{"line_number":406,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAADH%2F%2F%2B94%3D","line":403,"updated":"2012-01-03 17:07:44.000000000","message":"I think you want -H for header mode, this should eliminate the need for the grep.\n\nAlso, you need to specify -t snapshot (at least on OI 151)\n\nI think specifying \"-d 1\" as well is a good idea, then if you have a zfs volume that is a child of another zfs volume you won\u0027t see the child\u0027s snapshots.","commit_id":"3add4cdb0d783e6ce1326eb3e048eaf29a63e8d7"},{"author":{"_account_id":2256,"name":"Nilanjan Roy","email":"nilanjan1.roy@gmail.com","username":"nilanjan-r"},"change_message_id":"acf01f9816ce1ce484742d61edf444cd8a9923ad","unresolved":false,"context_lines":[{"line_number":400,"context_line":"        \"\"\"Checks for presense of snapshot of a volume\"\"\""},{"line_number":401,"context_line":"        zfs_poolname \u003d self._build_zfs_poolname(volume)"},{"line_number":402,"context_line":"        cmd \u003d (\"pfexec /usr/sbin/zfs list -r %s | \""},{"line_number":403,"context_line":"               \"grep %s| wc -l\" % (zfs_poolname, zfs_poolname))"},{"line_number":404,"context_line":"        (out, _err) \u003d self._run_ssh(cmd, check_exit_code\u003dFalse)"},{"line_number":405,"context_line":"        return int(out.strip())"},{"line_number":406,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAADH%2F%2F%2BtQ%3D","line":403,"in_reply_to":"AAAADH%2F%2F%2B94%3D","updated":"2012-01-05 07:15:05.000000000","message":"Yes using -H will eliminate the need for using grep. I shall change it and amend the commit.\n\n\"-d 1\"  should also work. \"-r\" option prints all the child datasets. However it should be noted that we can\u0027t create a zfs volume whose parent is another zfs volume. ZFS volume can be created only on ZFS filesystem.","commit_id":"3add4cdb0d783e6ce1326eb3e048eaf29a63e8d7"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"56b8751f44be76b7d5974f1ad369edb8fd2a2ab4","unresolved":false,"context_lines":[{"line_number":20,"context_line":"The unique thing about a SAN is that we don\u0027t expect that we can run the volume"},{"line_number":21,"context_line":"controller on the SAN hardware.  We expect to access it over SSH or some API."},{"line_number":22,"context_line":"\"\"\""},{"line_number":23,"context_line":"import os"},{"line_number":24,"context_line":"import paramiko"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"from xml.etree import ElementTree"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAD3%2F%2F9uQ%3D","line":23,"updated":"2012-01-16 21:21:16.000000000","message":"Why remove the blank line between the docstring an this import?","commit_id":"70cdec66f4faaeb166c319ecd65c777104269fb6"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"56b8751f44be76b7d5974f1ad369edb8fd2a2ab4","unresolved":false,"context_lines":[{"line_number":56,"context_line":"                    \u0027The ZFS path under which to create zvols for volumes.\u0027)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"#NOTE(nilanjan):This flag defines the zfs pool on the Solaris"},{"line_number":59,"context_line":"#storage that will be used to create nova volumes."},{"line_number":60,"context_line":"flags.DEFINE_string(\u0027poolname\u0027, \u0027rpool\u0027, \u0027ZFS pool name\u0027)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAD3%2F%2F9uE%3D","line":59,"updated":"2012-01-16 21:21:16.000000000","message":"I\u0027d much prefer if you included spaces between the \u0027#\u0027 and your comment text, i.e.:\n\n    # NOTE(nilanjan): This flag […]\n    # storage that […]","commit_id":"70cdec66f4faaeb166c319ecd65c777104269fb6"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"56b8751f44be76b7d5974f1ad369edb8fd2a2ab4","unresolved":false,"context_lines":[{"line_number":76,"context_line":"        return \"%s%s\" % (FLAGS.iscsi_target_prefix, volume[\u0027name\u0027])"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    # discover_volume is still OK"},{"line_number":79,"context_line":"    # undiscover_volume is still OK"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def _connect_to_ssh(self):"},{"line_number":82,"context_line":"        ssh \u003d paramiko.SSHClient()"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAD3%2F%2F9uA%3D","line":79,"updated":"2012-01-16 21:21:16.000000000","message":"I\u0027m not sure I understand what these comments mean; could you clarify?","commit_id":"70cdec66f4faaeb166c319ecd65c777104269fb6"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"371c1a6ee7f514754274573d2daa24ade295cf24","unresolved":false,"context_lines":[{"line_number":76,"context_line":"        return \"%s%s\" % (FLAGS.iscsi_target_prefix, volume[\u0027name\u0027])"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    # discover_volume is still OK"},{"line_number":79,"context_line":"    # undiscover_volume is still OK"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def _connect_to_ssh(self):"},{"line_number":82,"context_line":"        ssh \u003d paramiko.SSHClient()"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAD3%2F%2F9VA%3D","line":79,"in_reply_to":"AAAAD3%2F%2F9c8%3D","updated":"2012-01-17 16:40:50.000000000","message":"They\u0027re new comments, according to gerrit\u0027s diff.  They won\u0027t hurt anything, but it\u0027d probably be better to go ahead and remove them.","commit_id":"70cdec66f4faaeb166c319ecd65c777104269fb6"},{"author":{"_account_id":2256,"name":"Nilanjan Roy","email":"nilanjan1.roy@gmail.com","username":"nilanjan-r"},"change_message_id":"c26fd9ab37c0c87031e066c9e555a787972e44b7","unresolved":false,"context_lines":[{"line_number":76,"context_line":"        return \"%s%s\" % (FLAGS.iscsi_target_prefix, volume[\u0027name\u0027])"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    # discover_volume is still OK"},{"line_number":79,"context_line":"    # undiscover_volume is still OK"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def _connect_to_ssh(self):"},{"line_number":82,"context_line":"        ssh \u003d paramiko.SSHClient()"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAD3%2F%2F9c8%3D","line":79,"in_reply_to":"AAAAD3%2F%2F9uA%3D","updated":"2012-01-17 06:37:21.000000000","message":"I am not sure about this comment. It was there in previous version. If you want I shall remove it.","commit_id":"70cdec66f4faaeb166c319ecd65c777104269fb6"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"56b8751f44be76b7d5974f1ad369edb8fd2a2ab4","unresolved":false,"context_lines":[{"line_number":256,"context_line":""},{"line_number":257,"context_line":"    def _build_zfs_poolname(self, volume\u003dNone, snapshot\u003dNone):"},{"line_number":258,"context_line":"        \"\"\"Build zfs path\"\"\""},{"line_number":259,"context_line":"        #TODO(nilanjan):The variable name zfs_poolname"},{"line_number":260,"context_line":"        #should be replaced with zfs_volume"},{"line_number":261,"context_line":"        #as it sounds like a zfs pool (zpool)."},{"line_number":262,"context_line":"        #Also the function should be renamed to _build_zfs_path."}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAD3%2F%2F9t8%3D","line":259,"updated":"2012-01-16 21:21:16.000000000","message":"Again, spacing between \u0027#\u0027 and comment text would be nice, for readability...","commit_id":"70cdec66f4faaeb166c319ecd65c777104269fb6"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"56b8751f44be76b7d5974f1ad369edb8fd2a2ab4","unresolved":false,"context_lines":[{"line_number":394,"context_line":""},{"line_number":395,"context_line":"    def remove_export(self, context, volume):"},{"line_number":396,"context_line":"        \"\"\"Removes an export for a logical volume.\"\"\""},{"line_number":397,"context_line":"        #NOTE(nilanjan):Checks for presence of snapshot in the volume"},{"line_number":398,"context_line":"        snapshot_count \u003d self._get_snapshot_child_dataset_count(volume)"},{"line_number":399,"context_line":"        if snapshot_count \u003e 1:"},{"line_number":400,"context_line":"            #NOTE(nilanjan):If snapshot is present,"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAD3%2F%2F9t4%3D","line":397,"updated":"2012-01-16 21:21:16.000000000","message":"Again, spacing between \u0027#\u0027 and comment text…","commit_id":"70cdec66f4faaeb166c319ecd65c777104269fb6"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"56b8751f44be76b7d5974f1ad369edb8fd2a2ab4","unresolved":false,"context_lines":[{"line_number":434,"context_line":"        orig_snapshot_name \u003d self._build_zfs_poolname(None, snapshot)"},{"line_number":435,"context_line":"        dest_zfs_poolname \u003d self._build_zfs_poolname(volume)"},{"line_number":436,"context_line":"        dest_snapshot_name \u003d self._build_zfs_poolname(volume, snapshot)"},{"line_number":437,"context_line":"        #Creates a zfs volume from the snapshot"},{"line_number":438,"context_line":"        cmd_1 \u003d (\"pfexec /usr/sbin/zfs send %s | \""},{"line_number":439,"context_line":"                 \"pfexec /usr/sbin/zfs recv %s \" %"},{"line_number":440,"context_line":"                  (orig_snapshot_name, dest_zfs_poolname))"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAD3%2F%2F9t0%3D","line":437,"updated":"2012-01-16 21:21:16.000000000","message":"Again, spacing between \u0027#\u0027 and comment text, for readability","commit_id":"70cdec66f4faaeb166c319ecd65c777104269fb6"}]}