)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"2cfec90fad8a967a8918048f098f8916af6d7c97","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"02e5b9de_b7158bed","updated":"2025-11-19 13:54:31.000000000","message":"recheck - failed on unrelated store","commit_id":"66bff1ec9eed2cad03a48dac0721f5728bb68bd6"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f00edf7610d4a2e90a6c918249f6b42bc91580f4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"92a6b546_89c86c59","updated":"2025-12-15 15:25:58.000000000","message":"Thanks for working on this Fernando, few comments/concerns inline.","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"87e8677c761fca40bdfdae352d09110e53ca22af","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"ad17bb8e_d1019c19","updated":"2025-12-17 14:03:55.000000000","message":"recheck","commit_id":"64cb024ceaf0094c44eb733582149b46acfbfe29"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"46ded6d1b137ed160f9f7ace48eaf72487badb5e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"0cb58266_b6d48bdc","updated":"2025-12-22 06:48:07.000000000","message":"Thanks for addressing my comments, I\u0027ve additional comments inline","commit_id":"030071275333a09136352dd644fdbc5482d6498e"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"b9ff411a72cb9de3e4c61c0e0d80c10fb4f77803","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"f28f8876_aed91081","updated":"2025-12-18 14:19:47.000000000","message":"recheck","commit_id":"030071275333a09136352dd644fdbc5482d6498e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"153feba58fbae085d8326203d3df8be08fb010a3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"58713366_0ca02b77","updated":"2025-12-24 04:39:58.000000000","message":"Code and tests LGTM.  I think this approach makes sense.","commit_id":"7d083c10e48c9ec042a5c5d40e1184c6901451bb"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5306f29089763743d4126f20b69461927f8a4283","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"fc38f0db_23df51df","updated":"2025-12-24 07:59:55.000000000","message":"Sorry for catching some things late but i still feel one scenario needs to be covered when unmounting and there are a series of test scenarios I\u0027ve added that would be useful to assure better test coverage and minimum breakages when this code goes into deployments.","commit_id":"7d083c10e48c9ec042a5c5d40e1184c6901451bb"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"e6a66324471352029dbd1edfe1883998402c066c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"8bcb3123_28779002","updated":"2026-01-12 05:32:11.000000000","message":"Thanks for addressing my comments. Please mark the comments resolved to make sure they don\u0027t appear in latest PS.\nStill have one comment inline but I don\u0027t have a concrete evidence of it being a problem right now or the right permissions for this situation.\nCode and tests looks good.\nI was still expecting a case where unmounting fails and attachment count is 0 after which we perform another unmounting where unmounting succeeds but it might be too much for UTs and could be a manual verification.\nLGTM.","commit_id":"5711fe377294d2b780dc6b23bcc88852aeacaeb8"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"5b97ac1e627ac2104450105aa03bfe3f447778b1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"ac01d528_1659e89e","updated":"2026-03-25 14:42:22.000000000","message":"Looks good to go, please close older comments. Thank you!!","commit_id":"18207c1f514a716df845699a7638879a6878d4d3"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"cd0db791ef8dea6907949e403a673d6e4a86bb2a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"d7260eb7_3fa5883e","updated":"2026-04-01 13:19:29.000000000","message":"Thanks Fernando, closed out the comments pending from my side.","commit_id":"18207c1f514a716df845699a7638879a6878d4d3"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f04d1bfdbbfed188d4f67488bb6447ff5c27b927","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"01903767_86d32a4c","updated":"2026-03-24 13:45:41.000000000","message":"some \"unused\" part of the code was removed since my last review so still LGTM.","commit_id":"18207c1f514a716df845699a7638879a6878d4d3"}],"glance_store/common/fs_mount.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"f7b1a641035fa902423b535695c9b56c398590ff","unresolved":true,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        state \u003d {\"attachment_count\": 0}"},{"line_number":196,"context_line":"        try:"},{"line_number":197,"context_line":"            if os.path.exists(state_file):"},{"line_number":198,"context_line":"                with open(state_file, \u0027r\u0027) as f:"},{"line_number":199,"context_line":"                    state \u003d json.load(f)"},{"line_number":200,"context_line":"        except (IOError, ValueError, json.JSONDecodeError) as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"2ddd6a67_97f187c4","line":197,"updated":"2025-11-24 16:39:27.000000000","message":"Probably don\u0027t need to check exists() and then open() - just handle the failure from open.","commit_id":"66bff1ec9eed2cad03a48dac0721f5728bb68bd6"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"06152cfaefd5a0509ac1a9acc6e512793b6daa6b","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        state \u003d {\"attachment_count\": 0}"},{"line_number":196,"context_line":"        try:"},{"line_number":197,"context_line":"            if os.path.exists(state_file):"},{"line_number":198,"context_line":"                with open(state_file, \u0027r\u0027) as f:"},{"line_number":199,"context_line":"                    state \u003d json.load(f)"},{"line_number":200,"context_line":"        except (IOError, ValueError, json.JSONDecodeError) as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9b4315f4_16d534f5","line":197,"in_reply_to":"2ddd6a67_97f187c4","updated":"2025-12-16 15:44:36.000000000","message":"Agree. Changed as suggested.","commit_id":"66bff1ec9eed2cad03a48dac0721f5728bb68bd6"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"f7b1a641035fa902423b535695c9b56c398590ff","unresolved":true,"context_lines":[{"line_number":211,"context_line":"        try:"},{"line_number":212,"context_line":"            # Ensure directory exists"},{"line_number":213,"context_line":"            state_dir \u003d os.path.dirname(state_file)"},{"line_number":214,"context_line":"            if not os.path.exists(state_dir):"},{"line_number":215,"context_line":"                os.makedirs(state_dir, mode\u003d0o755, exist_ok\u003dTrue)"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"            # Write state atomically using a temporary file"}],"source_content_type":"text/x-python","patch_set":1,"id":"7023f465_7efe989c","line":214,"updated":"2025-11-24 16:39:27.000000000","message":"Don\u0027t need to check os.path.exists() here if passing exist_ok\u003dTrue?","commit_id":"66bff1ec9eed2cad03a48dac0721f5728bb68bd6"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"06152cfaefd5a0509ac1a9acc6e512793b6daa6b","unresolved":false,"context_lines":[{"line_number":211,"context_line":"        try:"},{"line_number":212,"context_line":"            # Ensure directory exists"},{"line_number":213,"context_line":"            state_dir \u003d os.path.dirname(state_file)"},{"line_number":214,"context_line":"            if not os.path.exists(state_dir):"},{"line_number":215,"context_line":"                os.makedirs(state_dir, mode\u003d0o755, exist_ok\u003dTrue)"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"            # Write state atomically using a temporary file"}],"source_content_type":"text/x-python","patch_set":1,"id":"ab8b3071_66a01b77","line":214,"in_reply_to":"7023f465_7efe989c","updated":"2025-12-16 15:44:36.000000000","message":"Agree. Changed as suggested.","commit_id":"66bff1ec9eed2cad03a48dac0721f5728bb68bd6"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f00edf7610d4a2e90a6c918249f6b42bc91580f4","unresolved":true,"context_lines":[{"line_number":59,"context_line":"    use_count \u003d 0"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    # Guards both state and use_count"},{"line_number":62,"context_line":"    cond \u003d threading.Condition()"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def __init__(self, host):"},{"line_number":65,"context_line":"        \"\"\"Initialise a new _HostMountState"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        We will block before creating a new state until all operations"},{"line_number":68,"context_line":"        using a previous state have completed."},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        :param host: host"},{"line_number":71,"context_line":"        \"\"\""},{"line_number":72,"context_line":"        # Wait until all operations using a previous state are"},{"line_number":73,"context_line":"        # complete before initialising a new one. Note that self.state is"},{"line_number":74,"context_line":"        # already None, set either by initialisation or by host_down. This"},{"line_number":75,"context_line":"        # means the current state will not be returned to any new callers,"},{"line_number":76,"context_line":"        # and use_count will eventually reach zero."},{"line_number":77,"context_line":"        # We do this to avoid a race between _HostMountState initialisation"},{"line_number":78,"context_line":"        # and an on-going mount/unmount operation"},{"line_number":79,"context_line":"        self.host \u003d host"},{"line_number":80,"context_line":"        with self.cond:"},{"line_number":81,"context_line":"            while self.use_count !\u003d 0:"},{"line_number":82,"context_line":"                self.cond.wait()"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"            # Another thread might have initialised state while we were"},{"line_number":85,"context_line":"            # waiting"},{"line_number":86,"context_line":"            if self.state is None:"},{"line_number":87,"context_line":"                LOG.debug(\u0027Initialising _HostMountState\u0027)"},{"line_number":88,"context_line":"                self.state \u003d _HostMountState()"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    @contextlib.contextmanager"},{"line_number":91,"context_line":"    def get_state(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"e2feb066_4c587139","line":88,"range":{"start_line":62,"start_character":0,"end_line":88,"end_character":46},"updated":"2025-12-15 15:25:58.000000000","message":"Do we still need the threading conditionals? It is possible to have multiple threads executing the operation inside same process but i feel it should be handled by the semaphore parameter provided in oslo_concurrency lock method[1]\n\n    semaphores – Container that provides semaphores to use when locking.\n    This ensures that threads inside the same application can not collide,\n    due to the fact that external process locks are unaware of a processes\n    active threads.\n    \nSo either:\n1. We don\u0027t need this at all\n2. If we need to prevent races in threads inside same process, semaphore seems to be a better way\n\n[1] https://docs.openstack.org/oslo.concurrency/latest/reference/lockutils.html#oslo_concurrency.lockutils.lock","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"c5442aeb992bb38fd237875e0eed60e3ef0e1c34","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    use_count \u003d 0"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    # Guards both state and use_count"},{"line_number":62,"context_line":"    cond \u003d threading.Condition()"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def __init__(self, host):"},{"line_number":65,"context_line":"        \"\"\"Initialise a new _HostMountState"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        We will block before creating a new state until all operations"},{"line_number":68,"context_line":"        using a previous state have completed."},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        :param host: host"},{"line_number":71,"context_line":"        \"\"\""},{"line_number":72,"context_line":"        # Wait until all operations using a previous state are"},{"line_number":73,"context_line":"        # complete before initialising a new one. Note that self.state is"},{"line_number":74,"context_line":"        # already None, set either by initialisation or by host_down. This"},{"line_number":75,"context_line":"        # means the current state will not be returned to any new callers,"},{"line_number":76,"context_line":"        # and use_count will eventually reach zero."},{"line_number":77,"context_line":"        # We do this to avoid a race between _HostMountState initialisation"},{"line_number":78,"context_line":"        # and an on-going mount/unmount operation"},{"line_number":79,"context_line":"        self.host \u003d host"},{"line_number":80,"context_line":"        with self.cond:"},{"line_number":81,"context_line":"            while self.use_count !\u003d 0:"},{"line_number":82,"context_line":"                self.cond.wait()"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"            # Another thread might have initialised state while we were"},{"line_number":85,"context_line":"            # waiting"},{"line_number":86,"context_line":"            if self.state is None:"},{"line_number":87,"context_line":"                LOG.debug(\u0027Initialising _HostMountState\u0027)"},{"line_number":88,"context_line":"                self.state \u003d _HostMountState()"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    @contextlib.contextmanager"},{"line_number":91,"context_line":"    def get_state(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"e0bd8bcb_9e9f9d14","line":88,"range":{"start_line":62,"start_character":0,"end_line":88,"end_character":46},"in_reply_to":"4ba9574f_18dbda26","updated":"2026-03-25 14:58:59.000000000","message":"Marking this as resolved since the referenced code has been removed.","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"983a5927fd5597adb0abc8c2dbcfff84bcd24e64","unresolved":true,"context_lines":[{"line_number":59,"context_line":"    use_count \u003d 0"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    # Guards both state and use_count"},{"line_number":62,"context_line":"    cond \u003d threading.Condition()"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def __init__(self, host):"},{"line_number":65,"context_line":"        \"\"\"Initialise a new _HostMountState"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        We will block before creating a new state until all operations"},{"line_number":68,"context_line":"        using a previous state have completed."},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        :param host: host"},{"line_number":71,"context_line":"        \"\"\""},{"line_number":72,"context_line":"        # Wait until all operations using a previous state are"},{"line_number":73,"context_line":"        # complete before initialising a new one. Note that self.state is"},{"line_number":74,"context_line":"        # already None, set either by initialisation or by host_down. This"},{"line_number":75,"context_line":"        # means the current state will not be returned to any new callers,"},{"line_number":76,"context_line":"        # and use_count will eventually reach zero."},{"line_number":77,"context_line":"        # We do this to avoid a race between _HostMountState initialisation"},{"line_number":78,"context_line":"        # and an on-going mount/unmount operation"},{"line_number":79,"context_line":"        self.host \u003d host"},{"line_number":80,"context_line":"        with self.cond:"},{"line_number":81,"context_line":"            while self.use_count !\u003d 0:"},{"line_number":82,"context_line":"                self.cond.wait()"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"            # Another thread might have initialised state while we were"},{"line_number":85,"context_line":"            # waiting"},{"line_number":86,"context_line":"            if self.state is None:"},{"line_number":87,"context_line":"                LOG.debug(\u0027Initialising _HostMountState\u0027)"},{"line_number":88,"context_line":"                self.state \u003d _HostMountState()"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    @contextlib.contextmanager"},{"line_number":91,"context_line":"    def get_state(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"0b32da60_7249d328","line":88,"range":{"start_line":62,"start_character":0,"end_line":88,"end_character":46},"in_reply_to":"e2feb066_4c587139","updated":"2025-12-17 20:26:29.000000000","message":"Good point Rajat! This is legacy code so I opted for removing the threading conditionals entirely. The actual mount/umount operations are now protected by oslo_concurrency.synchronized(), which handles both inter-process and intra-process coordination. \n\nAlso, _HostMountStateManager is initialized only once in glance_store code at module level (ln 343, `__manager__ \u003d _HostMountStateManager(HOST)`). This ensures __manager__ will be initialized only once on class import. \n\nHostMountStateManagerMeta() metaclass also ensures that _HostMountStateManager() instance is created in a singleton pattern.","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"abb5934a9c566f406f82ff1281117c9b552c2f95","unresolved":true,"context_lines":[{"line_number":59,"context_line":"    use_count \u003d 0"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    # Guards both state and use_count"},{"line_number":62,"context_line":"    cond \u003d threading.Condition()"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def __init__(self, host):"},{"line_number":65,"context_line":"        \"\"\"Initialise a new _HostMountState"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        We will block before creating a new state until all operations"},{"line_number":68,"context_line":"        using a previous state have completed."},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        :param host: host"},{"line_number":71,"context_line":"        \"\"\""},{"line_number":72,"context_line":"        # Wait until all operations using a previous state are"},{"line_number":73,"context_line":"        # complete before initialising a new one. Note that self.state is"},{"line_number":74,"context_line":"        # already None, set either by initialisation or by host_down. This"},{"line_number":75,"context_line":"        # means the current state will not be returned to any new callers,"},{"line_number":76,"context_line":"        # and use_count will eventually reach zero."},{"line_number":77,"context_line":"        # We do this to avoid a race between _HostMountState initialisation"},{"line_number":78,"context_line":"        # and an on-going mount/unmount operation"},{"line_number":79,"context_line":"        self.host \u003d host"},{"line_number":80,"context_line":"        with self.cond:"},{"line_number":81,"context_line":"            while self.use_count !\u003d 0:"},{"line_number":82,"context_line":"                self.cond.wait()"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"            # Another thread might have initialised state while we were"},{"line_number":85,"context_line":"            # waiting"},{"line_number":86,"context_line":"            if self.state is None:"},{"line_number":87,"context_line":"                LOG.debug(\u0027Initialising _HostMountState\u0027)"},{"line_number":88,"context_line":"                self.state \u003d _HostMountState()"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    @contextlib.contextmanager"},{"line_number":91,"context_line":"    def get_state(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"4ba9574f_18dbda26","line":88,"range":{"start_line":62,"start_character":0,"end_line":88,"end_character":46},"in_reply_to":"e2feb066_4c587139","updated":"2025-12-17 22:23:28.000000000","message":"Good point Rajat. I\u0027ve analized this part of the code more carefully and decided to remove threading.Condition() entirely.\n\nThat is because _HostMountStateManager() is initialized once via the singleton metaclass, even if","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f00edf7610d4a2e90a6c918249f6b42bc91580f4","unresolved":true,"context_lines":[{"line_number":135,"context_line":"    designing their own locking."},{"line_number":136,"context_line":"    \"\"\""},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    @staticmethod"},{"line_number":139,"context_line":"    def _get_safe_mountpoint_name(mountpoint):"},{"line_number":140,"context_line":"        \"\"\"Generate a safe name for the mountpoint."},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        Replaces unsafe characters with underscores and removes consecutive"},{"line_number":143,"context_line":"        underscores to create a safe filename."},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        :param mountpoint: The mountpoint name to sanitize"},{"line_number":146,"context_line":"        :returns: Sanitized mountpoint name"},{"line_number":147,"context_line":"        :rtype: str"},{"line_number":148,"context_line":"        \"\"\""},{"line_number":149,"context_line":"        # Replace non-word characters (except hyphens) with underscores"},{"line_number":150,"context_line":"        safe_name \u003d re.sub(r\u0027[^\\w\\-]\u0027, \u0027_\u0027, mountpoint)"},{"line_number":151,"context_line":"        # Replace consecutive underscores with single underscore"},{"line_number":152,"context_line":"        safe_name \u003d re.sub(r\u0027_+\u0027, \u0027_\u0027, safe_name)"},{"line_number":153,"context_line":"        # Strip leading/trailing underscores"},{"line_number":154,"context_line":"        return safe_name.strip(\u0027_\u0027)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    def _get_state_file_path(self, mountpoint):"},{"line_number":157,"context_line":"        \"\"\"Generate a state file path for the given mountpoint."}],"source_content_type":"text/x-python","patch_set":4,"id":"9d1f0c21_085e8a10","line":154,"range":{"start_line":138,"start_character":0,"end_line":154,"end_character":35},"updated":"2025-12-15 15:25:58.000000000","message":"I\u0027m not really sure if we need this. The mount point generated is a sha256 hash of the export path i.e. is a combination of numbers and letters so we should evaluate if the sanitation is even needed.\n\n    8db6edfa5db8c4a4b76e2ad4526a0f97806edb8c809ce63ffcdb8d1a551aa57a\n    Dec 15 14:26:23 rdhasman-devstack-nfs devstack@g-api.service[15993]: DEBUG oslo_concurrency.processutils [None req-7a13eb18-98d1-4abc-8c85-486f64d5b068 admin admin] CMD \"sudo glance-rootwrap /etc/glance/rootwrap.conf mount -t nfs localhost:/srv/nfs1 /opt/stack/data/glance/images/mnt/8db6edfa5db8c4a4b76e2ad4526a0f97806edb8c809ce63ffcdb8d1a551aa57a\" returned: 0 in 0.322s {{(pid\u003d15993) execute /opt/stack/data/venv/lib/python3.10/site-packages/oslo_concurrency/processutils.py:372}}","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"06152cfaefd5a0509ac1a9acc6e512793b6daa6b","unresolved":false,"context_lines":[{"line_number":135,"context_line":"    designing their own locking."},{"line_number":136,"context_line":"    \"\"\""},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    @staticmethod"},{"line_number":139,"context_line":"    def _get_safe_mountpoint_name(mountpoint):"},{"line_number":140,"context_line":"        \"\"\"Generate a safe name for the mountpoint."},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        Replaces unsafe characters with underscores and removes consecutive"},{"line_number":143,"context_line":"        underscores to create a safe filename."},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        :param mountpoint: The mountpoint name to sanitize"},{"line_number":146,"context_line":"        :returns: Sanitized mountpoint name"},{"line_number":147,"context_line":"        :rtype: str"},{"line_number":148,"context_line":"        \"\"\""},{"line_number":149,"context_line":"        # Replace non-word characters (except hyphens) with underscores"},{"line_number":150,"context_line":"        safe_name \u003d re.sub(r\u0027[^\\w\\-]\u0027, \u0027_\u0027, mountpoint)"},{"line_number":151,"context_line":"        # Replace consecutive underscores with single underscore"},{"line_number":152,"context_line":"        safe_name \u003d re.sub(r\u0027_+\u0027, \u0027_\u0027, safe_name)"},{"line_number":153,"context_line":"        # Strip leading/trailing underscores"},{"line_number":154,"context_line":"        return safe_name.strip(\u0027_\u0027)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    def _get_state_file_path(self, mountpoint):"},{"line_number":157,"context_line":"        \"\"\"Generate a state file path for the given mountpoint."}],"source_content_type":"text/x-python","patch_set":4,"id":"2f204c7f_e005e7f2","line":154,"range":{"start_line":138,"start_character":0,"end_line":154,"end_character":35},"in_reply_to":"9d1f0c21_085e8a10","updated":"2025-12-16 15:44:36.000000000","message":"Sanitizing the file state might be a more future proof aproach in case we decide to use the same code on other scenarios, but it also  unnecessarily increases complexity. I\u0027m removing the sanitization for both file state and lock names to keep the solution as simple as possible and tied to requirements.","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f00edf7610d4a2e90a6c918249f6b42bc91580f4","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        :param mountpoint: The mountpoint path"},{"line_number":213,"context_line":"        :param state: The state dictionary to save"},{"line_number":214,"context_line":"        \"\"\""},{"line_number":215,"context_line":"        state_file \u003d self._get_state_file_path(mountpoint)"},{"line_number":216,"context_line":"        temp_file \u003d state_file + \u0027.tmp\u0027"},{"line_number":217,"context_line":"        try:"},{"line_number":218,"context_line":"            # Write state atomically using a temporary file"},{"line_number":219,"context_line":"            with open(temp_file, \u0027w\u0027) as f:"},{"line_number":220,"context_line":"                json.dump(state, f)"},{"line_number":221,"context_line":"            os.rename(temp_file, state_file)"},{"line_number":222,"context_line":"        except FileNotFoundError:"},{"line_number":223,"context_line":"            # Directory doesn\u0027t exist, create it and retry"},{"line_number":224,"context_line":"            state_dir \u003d os.path.dirname(state_file)"}],"source_content_type":"text/x-python","patch_set":4,"id":"faa19270_8f84b974","line":221,"range":{"start_line":215,"start_character":0,"end_line":221,"end_character":44},"updated":"2025-12-15 15:25:58.000000000","message":"I\u0027m confused why we follow this strategy instead of just dumping directly into the main state file? Do we have any advantage or is it considered more \"safer\" than directly dumping things into the main file?","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5306f29089763743d4126f20b69461927f8a4283","unresolved":false,"context_lines":[{"line_number":212,"context_line":"        :param mountpoint: The mountpoint path"},{"line_number":213,"context_line":"        :param state: The state dictionary to save"},{"line_number":214,"context_line":"        \"\"\""},{"line_number":215,"context_line":"        state_file \u003d self._get_state_file_path(mountpoint)"},{"line_number":216,"context_line":"        temp_file \u003d state_file + \u0027.tmp\u0027"},{"line_number":217,"context_line":"        try:"},{"line_number":218,"context_line":"            # Write state atomically using a temporary file"},{"line_number":219,"context_line":"            with open(temp_file, \u0027w\u0027) as f:"},{"line_number":220,"context_line":"                json.dump(state, f)"},{"line_number":221,"context_line":"            os.rename(temp_file, state_file)"},{"line_number":222,"context_line":"        except FileNotFoundError:"},{"line_number":223,"context_line":"            # Directory doesn\u0027t exist, create it and retry"},{"line_number":224,"context_line":"            state_dir \u003d os.path.dirname(state_file)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9046d7be_44c0002b","line":221,"range":{"start_line":215,"start_character":0,"end_line":221,"end_character":44},"in_reply_to":"c0921d42_93366f3d","updated":"2025-12-24 07:59:55.000000000","message":"Acknowledged","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"06152cfaefd5a0509ac1a9acc6e512793b6daa6b","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        :param mountpoint: The mountpoint path"},{"line_number":213,"context_line":"        :param state: The state dictionary to save"},{"line_number":214,"context_line":"        \"\"\""},{"line_number":215,"context_line":"        state_file \u003d self._get_state_file_path(mountpoint)"},{"line_number":216,"context_line":"        temp_file \u003d state_file + \u0027.tmp\u0027"},{"line_number":217,"context_line":"        try:"},{"line_number":218,"context_line":"            # Write state atomically using a temporary file"},{"line_number":219,"context_line":"            with open(temp_file, \u0027w\u0027) as f:"},{"line_number":220,"context_line":"                json.dump(state, f)"},{"line_number":221,"context_line":"            os.rename(temp_file, state_file)"},{"line_number":222,"context_line":"        except FileNotFoundError:"},{"line_number":223,"context_line":"            # Directory doesn\u0027t exist, create it and retry"},{"line_number":224,"context_line":"            state_dir \u003d os.path.dirname(state_file)"}],"source_content_type":"text/x-python","patch_set":4,"id":"c0921d42_93366f3d","line":221,"range":{"start_line":215,"start_character":0,"end_line":221,"end_character":44},"in_reply_to":"faa19270_8f84b974","updated":"2025-12-16 15:44:36.000000000","message":"The reasoning for this apporach is that we only change the state file when we are sure the write operation completed. Suposing we write into the file directly, in case of a worker crashing or getting a SIGKILL in the midle of the write operation, it would leave the state file in a invalid state, and that would cause the other workers still alive to fail trying to parse a JSON state that was lost or partially written. The os.rename() ensures the state file is updated only when we are sure the write has completed.","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f00edf7610d4a2e90a6c918249f6b42bc91580f4","unresolved":true,"context_lines":[{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        mount_lock_name \u003d self._get_safe_lock_name(mountpoint)"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        @lockutils.synchronized(mount_lock_name, external\u003dTrue)"},{"line_number":266,"context_line":"        def _mount_with_lock():"},{"line_number":267,"context_line":"            if not os.path.ismount(mountpoint):"},{"line_number":268,"context_line":"                LOG.debug(\u0027Mounting %(mountpoint)s\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"f870e5e6_ee68aaa3","line":265,"range":{"start_line":265,"start_character":0,"end_line":265,"end_character":63},"updated":"2025-12-15 15:25:58.000000000","message":"one issue with placing the lock here is that we are doing a lot of things inside the critical section. Can we re-evaluate that mounting the NFS share requires locking (two processes simultaneously trying to mount a path) or at least get to know how much time does it take to execute this whole method, there should be a time associated with the debug log for this lock.","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"cd0db791ef8dea6907949e403a673d6e4a86bb2a","unresolved":false,"context_lines":[{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        mount_lock_name \u003d self._get_safe_lock_name(mountpoint)"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        @lockutils.synchronized(mount_lock_name, external\u003dTrue)"},{"line_number":266,"context_line":"        def _mount_with_lock():"},{"line_number":267,"context_line":"            if not os.path.ismount(mountpoint):"},{"line_number":268,"context_line":"                LOG.debug(\u0027Mounting %(mountpoint)s\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"419d17b0_d0e73a62","line":265,"range":{"start_line":265,"start_character":0,"end_line":265,"end_character":63},"in_reply_to":"3b8f7ad1_5e6a8ae4","updated":"2026-04-01 13:19:29.000000000","message":"Thanks for the logs, at least we have some idea about something that is not blocking for a long time even in a minimal env like devstack. (real deployments will obviously acquire it for longer duration due to the distributed nature but that\u0027s expected).","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"abb5934a9c566f406f82ff1281117c9b552c2f95","unresolved":true,"context_lines":[{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        mount_lock_name \u003d self._get_safe_lock_name(mountpoint)"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        @lockutils.synchronized(mount_lock_name, external\u003dTrue)"},{"line_number":266,"context_line":"        def _mount_with_lock():"},{"line_number":267,"context_line":"            if not os.path.ismount(mountpoint):"},{"line_number":268,"context_line":"                LOG.debug(\u0027Mounting %(mountpoint)s\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"b4e2939d_5c02ab09","line":265,"range":{"start_line":265,"start_character":0,"end_line":265,"end_character":63},"in_reply_to":"a13a10a1_358c1916","updated":"2025-12-17 22:23:28.000000000","message":"The results will vary a lot depending on network and nfs performance. \n\nWe could evaluate the possibility of always trying to perform the system mount operation despite the counter. I don\u0027t know this is safe tho, as some mount commands will inevitably fail due to race conditions.","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"c5442aeb992bb38fd237875e0eed60e3ef0e1c34","unresolved":false,"context_lines":[{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        mount_lock_name \u003d self._get_safe_lock_name(mountpoint)"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        @lockutils.synchronized(mount_lock_name, external\u003dTrue)"},{"line_number":266,"context_line":"        def _mount_with_lock():"},{"line_number":267,"context_line":"            if not os.path.ismount(mountpoint):"},{"line_number":268,"context_line":"                LOG.debug(\u0027Mounting %(mountpoint)s\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3b8f7ad1_5e6a8ae4","line":265,"range":{"start_line":265,"start_character":0,"end_line":265,"end_character":63},"in_reply_to":"b4e2939d_5c02ab09","updated":"2026-03-25 14:58:59.000000000","message":"Marking as resolved. @rajatdhasmana, feel free to reopen if you need further clarification.","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"871c051ee031d171ddb036d3f214bcea6694067b","unresolved":true,"context_lines":[{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        mount_lock_name \u003d self._get_safe_lock_name(mountpoint)"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        @lockutils.synchronized(mount_lock_name, external\u003dTrue)"},{"line_number":266,"context_line":"        def _mount_with_lock():"},{"line_number":267,"context_line":"            if not os.path.ismount(mountpoint):"},{"line_number":268,"context_line":"                LOG.debug(\u0027Mounting %(mountpoint)s\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"a13a10a1_358c1916","line":265,"range":{"start_line":265,"start_character":0,"end_line":265,"end_character":63},"in_reply_to":"f870e5e6_ee68aaa3","updated":"2025-12-17 22:10:29.000000000","message":"I\u0027ve got some logs from my test environment. worker-1 took 0.342s to complete and release the lock, while worker-2 acquired and relesed the lock much faster as it just skipped the mount operation.\n\nworker 1 (pid 1068980)\n\n```\nDec 17 21:45:46 devstack-7fb676c9-8854-4f24-91d6-76794eb3f0fd devstack@g-api.service[1068980]: DEBUG oslo_concurrency.lockutils [None req-8a975e49-14f6-4a28-9a41-03d23eda2035 admin admin] Lock \"fs_mount_8db6edfa5db8c4a4b76e2ad4526a0f97806edb8c809ce63ffcdb8d1a551aa57a\" \"released\" by \"glance_store.common.fs_mount._HostMountState.mount.\u003clocals\u003e._mount_with_lock\" :: held 0.342s {{(pid\u003d1068980) inner /opt/stack/data/venv/lib/python3.12/site-packages/oslo_concurrency/lockutils.py:424}}\n```\n\nworker 2 (pid 1068979)\n\n```\nDec 17 21:45:46 devstack-7fb676c9-8854-4f24-91d6-76794eb3f0fd devstack@g-api.service[1068979]: DEBUG oslo_concurrency.lockutils [None req-6f61f57d-7a59-4da0-99ba-959467f54366 admin admin] Lock \"fs_mount_8db6edfa5db8c4a4b76e2ad4526a0f97806edb8c809ce63ffcdb8d1a551aa57a\" \"released\" by \"glance_store.common.fs_mount._HostMountState.mount.\u003clocals\u003e._mount_with_lock\" :: held 0.001s {{(pid\u003d1068979) inner /opt/stack/data/venv/lib/python3.12/site-packages/oslo_concurrency/lockutils.py:424}}\n```","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f00edf7610d4a2e90a6c918249f6b42bc91580f4","unresolved":true,"context_lines":[{"line_number":300,"context_line":""},{"line_number":301,"context_line":"            state \u003d self._get_state(mountpoint)"},{"line_number":302,"context_line":"            # Increment attachment count and add metadata"},{"line_number":303,"context_line":"            state[\"attachment_count\"] \u003d state.get(\"attachment_count\", 0) + 1"},{"line_number":304,"context_line":"            # Add metadata if not already present"},{"line_number":305,"context_line":"            if \"mountpoint\" not in state:"},{"line_number":306,"context_line":"                state[\"mountpoint\"] \u003d mountpoint"}],"source_content_type":"text/x-python","patch_set":4,"id":"4c87fa9a_86fdc088","line":303,"range":{"start_line":303,"start_character":40,"end_line":303,"end_character":72},"updated":"2025-12-15 15:25:58.000000000","message":"I\u0027m skeptical if the json.load correctly translates the string data in the file to proper integer\nCan we quickly create a small program to verify it? and if possible can we rewrite this as the following just to be safe,\n\n    state[\"attachment_count\"] \u003d int(state.get(\"attachment_count\", 0)) + 1","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"06152cfaefd5a0509ac1a9acc6e512793b6daa6b","unresolved":false,"context_lines":[{"line_number":300,"context_line":""},{"line_number":301,"context_line":"            state \u003d self._get_state(mountpoint)"},{"line_number":302,"context_line":"            # Increment attachment count and add metadata"},{"line_number":303,"context_line":"            state[\"attachment_count\"] \u003d state.get(\"attachment_count\", 0) + 1"},{"line_number":304,"context_line":"            # Add metadata if not already present"},{"line_number":305,"context_line":"            if \"mountpoint\" not in state:"},{"line_number":306,"context_line":"                state[\"mountpoint\"] \u003d mountpoint"}],"source_content_type":"text/x-python","patch_set":4,"id":"01e2e3b3_17e08004","line":303,"range":{"start_line":303,"start_character":40,"end_line":303,"end_character":72},"in_reply_to":"4c87fa9a_86fdc088","updated":"2025-12-16 15:44:36.000000000","message":"I believe the json.load is garanteed to parse the data as integer. The only possible scenario where it might fail would be if we mistakenly set the state as `\"attachment_count\": \"5\"` instead of  `\"attachment_count\": 5`. I\u0027m changing the code to include this as a safe measure, but will centralize the integer parse in the _get_sate() metthod to avoid doing the cast on multiple places.","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f00edf7610d4a2e90a6c918249f6b42bc91580f4","unresolved":true,"context_lines":[{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        mount_lock_name \u003d self._get_safe_lock_name(mountpoint)"},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"        @lockutils.synchronized(mount_lock_name, external\u003dTrue)"},{"line_number":334,"context_line":"        def _umount_with_lock():"},{"line_number":335,"context_line":"            state \u003d self._get_state(mountpoint)"},{"line_number":336,"context_line":"            current_count \u003d state.get(\"attachment_count\", 0)"}],"source_content_type":"text/x-python","patch_set":4,"id":"541a1526_ec8697e4","line":333,"range":{"start_line":333,"start_character":0,"end_line":333,"end_character":63},"updated":"2025-12-15 15:25:58.000000000","message":"can we get the time required to execute this method? should be visible in the logs when the lock is released.\nI\u0027m just trying to think if the mount/umount command gets stuck and could lead to hanging of the glance service and if anything we can do about it","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"cd0db791ef8dea6907949e403a673d6e4a86bb2a","unresolved":false,"context_lines":[{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        mount_lock_name \u003d self._get_safe_lock_name(mountpoint)"},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"        @lockutils.synchronized(mount_lock_name, external\u003dTrue)"},{"line_number":334,"context_line":"        def _umount_with_lock():"},{"line_number":335,"context_line":"            state \u003d self._get_state(mountpoint)"},{"line_number":336,"context_line":"            current_count \u003d state.get(\"attachment_count\", 0)"}],"source_content_type":"text/x-python","patch_set":4,"id":"41ac1c31_63eb343d","line":333,"range":{"start_line":333,"start_character":0,"end_line":333,"end_character":63},"in_reply_to":"2c8e6b11_db33ca96","updated":"2026-04-01 13:19:29.000000000","message":"Yes, the query is answered and logs provided in L#265 are satisfactory from my perspective.","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"accb443fc9d00734f3f31a538aadbd8e26541227","unresolved":false,"context_lines":[{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        mount_lock_name \u003d self._get_safe_lock_name(mountpoint)"},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"        @lockutils.synchronized(mount_lock_name, external\u003dTrue)"},{"line_number":334,"context_line":"        def _umount_with_lock():"},{"line_number":335,"context_line":"            state \u003d self._get_state(mountpoint)"},{"line_number":336,"context_line":"            current_count \u003d state.get(\"attachment_count\", 0)"}],"source_content_type":"text/x-python","patch_set":4,"id":"2c8e6b11_db33ca96","line":333,"range":{"start_line":333,"start_character":0,"end_line":333,"end_character":63},"in_reply_to":"541a1526_ec8697e4","updated":"2026-03-25 16:38:20.000000000","message":"This is the same as line #265. I\u0027m marking it as resolved, but @rajatdhasmana@gmail.com feel free to reopen it if your question hasn\u0027t been properly clarified.","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f00edf7610d4a2e90a6c918249f6b42bc91580f4","unresolved":true,"context_lines":[{"line_number":333,"context_line":"        @lockutils.synchronized(mount_lock_name, external\u003dTrue)"},{"line_number":334,"context_line":"        def _umount_with_lock():"},{"line_number":335,"context_line":"            state \u003d self._get_state(mountpoint)"},{"line_number":336,"context_line":"            current_count \u003d state.get(\"attachment_count\", 0)"},{"line_number":337,"context_line":"            if current_count \u003d\u003d 0:"},{"line_number":338,"context_line":"                LOG.warning(_LW(\"Request to remove attachment \""},{"line_number":339,"context_line":"                                \"(%(vol_name)s, %(host)s) from \""}],"source_content_type":"text/x-python","patch_set":4,"id":"e5b694ee_8cfa4957","line":336,"range":{"start_line":336,"start_character":28,"end_line":336,"end_character":60},"updated":"2025-12-15 15:25:58.000000000","message":"same concern, are we really converting the \"string\" numbers to \"integer\" properly?\nThis looks much safer,\n\n    current_count \u003d int(state.get(\"attachment_count\", 0))","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"06152cfaefd5a0509ac1a9acc6e512793b6daa6b","unresolved":false,"context_lines":[{"line_number":333,"context_line":"        @lockutils.synchronized(mount_lock_name, external\u003dTrue)"},{"line_number":334,"context_line":"        def _umount_with_lock():"},{"line_number":335,"context_line":"            state \u003d self._get_state(mountpoint)"},{"line_number":336,"context_line":"            current_count \u003d state.get(\"attachment_count\", 0)"},{"line_number":337,"context_line":"            if current_count \u003d\u003d 0:"},{"line_number":338,"context_line":"                LOG.warning(_LW(\"Request to remove attachment \""},{"line_number":339,"context_line":"                                \"(%(vol_name)s, %(host)s) from \""}],"source_content_type":"text/x-python","patch_set":4,"id":"abaed36d_5691b369","line":336,"range":{"start_line":336,"start_character":28,"end_line":336,"end_character":60},"in_reply_to":"e5b694ee_8cfa4957","updated":"2025-12-16 15:44:36.000000000","message":"I\u0027m changing the code to include this as a safe measure, I will centralize the integer casting in the _get_sate() metthod to avoid doing the cast on multiple places.","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f00edf7610d4a2e90a6c918249f6b42bc91580f4","unresolved":true,"context_lines":[{"line_number":362,"context_line":"                    except OSError as e:"},{"line_number":363,"context_line":"                        LOG.warning(\"Failed to remove state file %s: %s\","},{"line_number":364,"context_line":"                                    state_file, e)"},{"line_number":365,"context_line":"                else:"},{"line_number":366,"context_line":"                    # update state despite failed to unmount"},{"line_number":367,"context_line":"                    self._save_state(mountpoint, state)"},{"line_number":368,"context_line":"            else:"},{"line_number":369,"context_line":"                # Still has attachments, update state"},{"line_number":370,"context_line":"                self._save_state(mountpoint, state)"}],"source_content_type":"text/x-python","patch_set":4,"id":"39e4a312_f2a0ed8b","line":367,"range":{"start_line":365,"start_character":0,"end_line":367,"end_character":55},"updated":"2025-12-15 15:25:58.000000000","message":"This could lead to weird behavior where we fail to unmount and update the state file with attachment_count\u003d0 but the share is still mounted","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"06152cfaefd5a0509ac1a9acc6e512793b6daa6b","unresolved":true,"context_lines":[{"line_number":362,"context_line":"                    except OSError as e:"},{"line_number":363,"context_line":"                        LOG.warning(\"Failed to remove state file %s: %s\","},{"line_number":364,"context_line":"                                    state_file, e)"},{"line_number":365,"context_line":"                else:"},{"line_number":366,"context_line":"                    # update state despite failed to unmount"},{"line_number":367,"context_line":"                    self._save_state(mountpoint, state)"},{"line_number":368,"context_line":"            else:"},{"line_number":369,"context_line":"                # Still has attachments, update state"},{"line_number":370,"context_line":"                self._save_state(mountpoint, state)"}],"source_content_type":"text/x-python","patch_set":4,"id":"e4579f0b_807feb13","line":367,"range":{"start_line":365,"start_character":0,"end_line":367,"end_character":55},"in_reply_to":"39e4a312_f2a0ed8b","updated":"2025-12-16 15:44:36.000000000","message":"I think this is important because if we don\u0027t drop the counter in case of a umount failure, other workers won\u0027t have the change to try umounting the mountpoint later. \n\nKeeping the counter untouched in case of a failure will result in a invalid state, as the worker already completed its operation despite the mount still exists. IMO dropping the counter despite of possible errors is the safest aproach to keep state consistent.","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"c5442aeb992bb38fd237875e0eed60e3ef0e1c34","unresolved":false,"context_lines":[{"line_number":362,"context_line":"                    except OSError as e:"},{"line_number":363,"context_line":"                        LOG.warning(\"Failed to remove state file %s: %s\","},{"line_number":364,"context_line":"                                    state_file, e)"},{"line_number":365,"context_line":"                else:"},{"line_number":366,"context_line":"                    # update state despite failed to unmount"},{"line_number":367,"context_line":"                    self._save_state(mountpoint, state)"},{"line_number":368,"context_line":"            else:"},{"line_number":369,"context_line":"                # Still has attachments, update state"},{"line_number":370,"context_line":"                self._save_state(mountpoint, state)"}],"source_content_type":"text/x-python","patch_set":4,"id":"10ff224e_febbb9cb","line":367,"range":{"start_line":365,"start_character":0,"end_line":367,"end_character":55},"in_reply_to":"46b3beee_01b26bab","updated":"2026-03-25 14:58:59.000000000","message":"This has been addressed in the comments section from ln #215.","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5306f29089763743d4126f20b69461927f8a4283","unresolved":true,"context_lines":[{"line_number":362,"context_line":"                    except OSError as e:"},{"line_number":363,"context_line":"                        LOG.warning(\"Failed to remove state file %s: %s\","},{"line_number":364,"context_line":"                                    state_file, e)"},{"line_number":365,"context_line":"                else:"},{"line_number":366,"context_line":"                    # update state despite failed to unmount"},{"line_number":367,"context_line":"                    self._save_state(mountpoint, state)"},{"line_number":368,"context_line":"            else:"},{"line_number":369,"context_line":"                # Still has attachments, update state"},{"line_number":370,"context_line":"                self._save_state(mountpoint, state)"}],"source_content_type":"text/x-python","patch_set":4,"id":"46b3beee_01b26bab","line":367,"range":{"start_line":365,"start_character":0,"end_line":367,"end_character":55},"in_reply_to":"e4579f0b_807feb13","updated":"2025-12-24 07:59:55.000000000","message":"I\u0027ve added a comment on L#215-223 where we should handle this case","commit_id":"de748d4c8fa7e3019fb97cb4d19ede160432e6ba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"46ded6d1b137ed160f9f7ace48eaf72487badb5e","unresolved":true,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"HOST \u003d socket.gethostname()"},{"line_number":30,"context_line":"CONF \u003d cfg.CONF"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"2e4a00f9_069e5a1c","line":29,"range":{"start_line":29,"start_character":0,"end_line":29,"end_character":27},"updated":"2025-12-22 06:48:07.000000000","message":"This is only used for exception case logging, I don\u0027t think this is required anymore","commit_id":"030071275333a09136352dd644fdbc5482d6498e"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"29e1f92432d828c8778af4eea15c61ebbd4bdee3","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"HOST \u003d socket.gethostname()"},{"line_number":30,"context_line":"CONF \u003d cfg.CONF"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"c7e71737_2d1cf4a8","line":29,"range":{"start_line":29,"start_character":0,"end_line":29,"end_character":27},"in_reply_to":"2e4a00f9_069e5a1c","updated":"2025-12-23 17:46:09.000000000","message":"Done","commit_id":"030071275333a09136352dd644fdbc5482d6498e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"46ded6d1b137ed160f9f7ace48eaf72487badb5e","unresolved":true,"context_lines":[{"line_number":40,"context_line":"        return cls._instance[cls]"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"class _HostMountStateManager(metaclass\u003dHostMountStateManagerMeta):"},{"line_number":44,"context_line":"    \"\"\"A global manager of filesystem mounts."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    _HostMountStateManager manages a _HostMountState object for the current"}],"source_content_type":"text/x-python","patch_set":7,"id":"f025eab7_fe66f47b","line":43,"range":{"start_line":43,"start_character":0,"end_line":43,"end_character":66},"updated":"2025-12-22 06:48:07.000000000","message":"given my other comments, probably we can remove this concept as I don\u0027t see any usage of this anymore","commit_id":"030071275333a09136352dd644fdbc5482d6498e"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"29e1f92432d828c8778af4eea15c61ebbd4bdee3","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        return cls._instance[cls]"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"class _HostMountStateManager(metaclass\u003dHostMountStateManagerMeta):"},{"line_number":44,"context_line":"    \"\"\"A global manager of filesystem mounts."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    _HostMountStateManager manages a _HostMountState object for the current"}],"source_content_type":"text/x-python","patch_set":7,"id":"3ba1b52b_63374efe","line":43,"range":{"start_line":43,"start_character":0,"end_line":43,"end_character":66},"in_reply_to":"f025eab7_fe66f47b","updated":"2025-12-23 17:46:09.000000000","message":"Yeah, I agree. This was intented to enforce only a single instance of _HostMountState is created, even if other _HostMountState objects got initialized in different places in the code. As current implementation is designed to be initialized only once in this module during import, this is no longer necessary.","commit_id":"030071275333a09136352dd644fdbc5482d6498e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"46ded6d1b137ed160f9f7ace48eaf72487badb5e","unresolved":true,"context_lines":[{"line_number":50,"context_line":"    The metaclass ensures this is a singleton - multiple calls to instantiate"},{"line_number":51,"context_line":"    this class will return the same instance object. Per-mountpoint operations"},{"line_number":52,"context_line":"    are protected by oslo_concurrency locks which handle both inter-process"},{"line_number":53,"context_line":"    and intra-process (multi-threaded) synchronization."},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    \"\"\""},{"line_number":56,"context_line":"    state \u003d None"}],"source_content_type":"text/x-python","patch_set":7,"id":"736ac521_f5ae2b6e","line":53,"range":{"start_line":53,"start_character":4,"end_line":53,"end_character":55},"updated":"2025-12-22 06:48:07.000000000","message":"this is not entirely true, IMO it just handles the inter-process part.","commit_id":"030071275333a09136352dd644fdbc5482d6498e"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"29e1f92432d828c8778af4eea15c61ebbd4bdee3","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    The metaclass ensures this is a singleton - multiple calls to instantiate"},{"line_number":51,"context_line":"    this class will return the same instance object. Per-mountpoint operations"},{"line_number":52,"context_line":"    are protected by oslo_concurrency locks which handle both inter-process"},{"line_number":53,"context_line":"    and intra-process (multi-threaded) synchronization."},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    \"\"\""},{"line_number":56,"context_line":"    state \u003d None"}],"source_content_type":"text/x-python","patch_set":7,"id":"00029a0c_03541030","line":53,"range":{"start_line":53,"start_character":4,"end_line":53,"end_character":55},"in_reply_to":"736ac521_f5ae2b6e","updated":"2025-12-23 17:46:09.000000000","message":"Class an docstrings have been removed.","commit_id":"030071275333a09136352dd644fdbc5482d6498e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"46ded6d1b137ed160f9f7ace48eaf72487badb5e","unresolved":true,"context_lines":[{"line_number":65,"context_line":"            LOG.debug(\u0027Initialising _HostMountState\u0027)"},{"line_number":66,"context_line":"            self.state \u003d _HostMountState()"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    @contextlib.contextmanager"},{"line_number":69,"context_line":"    def get_state(self):"},{"line_number":70,"context_line":"        \"\"\"Return the current mount state."},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        :rtype: _HostMountState"},{"line_number":73,"context_line":"        \"\"\""},{"line_number":74,"context_line":"        if self.state is None:"},{"line_number":75,"context_line":"            LOG.error(\u0027Host not initialized\u0027)"},{"line_number":76,"context_line":"            raise exceptions.HostNotInitialized(host\u003dself.host)"},{"line_number":77,"context_line":"        LOG.debug(\u0027Got _HostMountState\u0027)"},{"line_number":78,"context_line":"        yield self.state"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"class _HostMountState(object):"}],"source_content_type":"text/x-python","patch_set":7,"id":"c59d5fe9_2ccc3023","line":78,"range":{"start_line":68,"start_character":0,"end_line":78,"end_character":24},"updated":"2025-12-22 06:48:07.000000000","message":"This doesn\u0027t seem to be needed given we handle the locking in a different way now","commit_id":"030071275333a09136352dd644fdbc5482d6498e"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"29e1f92432d828c8778af4eea15c61ebbd4bdee3","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            LOG.debug(\u0027Initialising _HostMountState\u0027)"},{"line_number":66,"context_line":"            self.state \u003d _HostMountState()"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    @contextlib.contextmanager"},{"line_number":69,"context_line":"    def get_state(self):"},{"line_number":70,"context_line":"        \"\"\"Return the current mount state."},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        :rtype: _HostMountState"},{"line_number":73,"context_line":"        \"\"\""},{"line_number":74,"context_line":"        if self.state is None:"},{"line_number":75,"context_line":"            LOG.error(\u0027Host not initialized\u0027)"},{"line_number":76,"context_line":"            raise exceptions.HostNotInitialized(host\u003dself.host)"},{"line_number":77,"context_line":"        LOG.debug(\u0027Got _HostMountState\u0027)"},{"line_number":78,"context_line":"        yield self.state"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"class _HostMountState(object):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3cab16c4_4cd547a1","line":78,"range":{"start_line":68,"start_character":0,"end_line":78,"end_character":24},"in_reply_to":"c59d5fe9_2ccc3023","updated":"2025-12-23 17:46:09.000000000","message":"This has been removed as mentioned above.","commit_id":"030071275333a09136352dd644fdbc5482d6498e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"46ded6d1b137ed160f9f7ace48eaf72487badb5e","unresolved":true,"context_lines":[{"line_number":343,"context_line":"          options\u003dNone):"},{"line_number":344,"context_line":"    \"\"\"A convenience wrapper around _HostMountState.mount()\"\"\""},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"    with __manager__.get_state() as mount_state:"},{"line_number":347,"context_line":"        mount_state.mount(fstype, export, vol_name, mountpoint, host,"},{"line_number":348,"context_line":"                          rootwrap_helper, options)"},{"line_number":349,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fec1243a_54f85d4a","line":346,"range":{"start_line":346,"start_character":0,"end_line":346,"end_character":48},"updated":"2025-12-22 06:48:07.000000000","message":"i don\u0027t think this is needed anymore given now the locking happens inside the mount operation itself","commit_id":"030071275333a09136352dd644fdbc5482d6498e"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"29e1f92432d828c8778af4eea15c61ebbd4bdee3","unresolved":false,"context_lines":[{"line_number":343,"context_line":"          options\u003dNone):"},{"line_number":344,"context_line":"    \"\"\"A convenience wrapper around _HostMountState.mount()\"\"\""},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"    with __manager__.get_state() as mount_state:"},{"line_number":347,"context_line":"        mount_state.mount(fstype, export, vol_name, mountpoint, host,"},{"line_number":348,"context_line":"                          rootwrap_helper, options)"},{"line_number":349,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"0af530d0_7e970b5e","line":346,"range":{"start_line":346,"start_character":0,"end_line":346,"end_character":48},"in_reply_to":"fec1243a_54f85d4a","updated":"2025-12-23 17:46:09.000000000","message":"This has been removed as mentioned above.","commit_id":"030071275333a09136352dd644fdbc5482d6498e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"46ded6d1b137ed160f9f7ace48eaf72487badb5e","unresolved":true,"context_lines":[{"line_number":351,"context_line":"def umount(vol_name, mountpoint, host, rootwrap_helper):"},{"line_number":352,"context_line":"    \"\"\"A convenience wrapper around _HostMountState.umount()\"\"\""},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    with __manager__.get_state() as mount_state:"},{"line_number":355,"context_line":"        mount_state.umount(vol_name, mountpoint, host, rootwrap_helper)"}],"source_content_type":"text/x-python","patch_set":7,"id":"f102c81f_e14f6a00","line":354,"range":{"start_line":354,"start_character":0,"end_line":354,"end_character":48},"updated":"2025-12-22 06:48:07.000000000","message":"same","commit_id":"030071275333a09136352dd644fdbc5482d6498e"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"29e1f92432d828c8778af4eea15c61ebbd4bdee3","unresolved":false,"context_lines":[{"line_number":351,"context_line":"def umount(vol_name, mountpoint, host, rootwrap_helper):"},{"line_number":352,"context_line":"    \"\"\"A convenience wrapper around _HostMountState.umount()\"\"\""},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    with __manager__.get_state() as mount_state:"},{"line_number":355,"context_line":"        mount_state.umount(vol_name, mountpoint, host, rootwrap_helper)"}],"source_content_type":"text/x-python","patch_set":7,"id":"23faf0a4_0803f2a0","line":354,"range":{"start_line":354,"start_character":0,"end_line":354,"end_character":48},"in_reply_to":"f102c81f_e14f6a00","updated":"2025-12-23 17:46:09.000000000","message":"This has been removed as mentioned above.","commit_id":"030071275333a09136352dd644fdbc5482d6498e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5306f29089763743d4126f20b69461927f8a4283","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        state_dir \u003d os.path.join(os.path.dirname(mountpoint), \u0027_state\u0027)"},{"line_number":57,"context_line":"        # Extract the mountpoint name from the full path in SHA256 hash format"},{"line_number":58,"context_line":"        mountpoint_name \u003d os.path.basename(mountpoint)"},{"line_number":59,"context_line":"        state_file \u003d os.path.join(state_dir,"},{"line_number":60,"context_line":"                                  \u0027mount_state_%s.json\u0027 % mountpoint_name)"},{"line_number":61,"context_line":"        return state_file"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def _get_state(self, mountpoint):"}],"source_content_type":"text/x-python","patch_set":8,"id":"9d49d71e_39267c73","line":60,"range":{"start_line":59,"start_character":21,"end_line":60,"end_character":74},"updated":"2025-12-24 07:59:55.000000000","message":"so eventually we will end up with a path like\n\n    \u003cmountpoint_dir\u003e/_state/mount_state_\u003cmountpoint_hash\u003e.json\n\nI\u0027m wondering if we need the _state directory but its not a blocker","commit_id":"7d083c10e48c9ec042a5c5d40e1184c6901451bb"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5306f29089763743d4126f20b69461927f8a4283","unresolved":true,"context_lines":[{"line_number":102,"context_line":"        except FileNotFoundError:"},{"line_number":103,"context_line":"            # Directory doesn\u0027t exist, create it and retry"},{"line_number":104,"context_line":"            state_dir \u003d os.path.dirname(state_file)"},{"line_number":105,"context_line":"            os.makedirs(state_dir, mode\u003d0o755, exist_ok\u003dTrue)"},{"line_number":106,"context_line":"            with open(temp_file, \u0027w\u0027) as f:"},{"line_number":107,"context_line":"                json.dump(state, f)"},{"line_number":108,"context_line":"            os.rename(temp_file, state_file)"}],"source_content_type":"text/x-python","patch_set":8,"id":"44d8454d_ebc19274","line":105,"range":{"start_line":105,"start_character":40,"end_line":105,"end_character":45},"updated":"2025-12-24 07:59:55.000000000","message":"Need to check if the groups and others permissions are useful or not","commit_id":"7d083c10e48c9ec042a5c5d40e1184c6901451bb"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"2e22c78e593872291311d7b34fc5a65cb0f48d09","unresolved":true,"context_lines":[{"line_number":102,"context_line":"        except FileNotFoundError:"},{"line_number":103,"context_line":"            # Directory doesn\u0027t exist, create it and retry"},{"line_number":104,"context_line":"            state_dir \u003d os.path.dirname(state_file)"},{"line_number":105,"context_line":"            os.makedirs(state_dir, mode\u003d0o755, exist_ok\u003dTrue)"},{"line_number":106,"context_line":"            with open(temp_file, \u0027w\u0027) as f:"},{"line_number":107,"context_line":"                json.dump(state, f)"},{"line_number":108,"context_line":"            os.rename(temp_file, state_file)"}],"source_content_type":"text/x-python","patch_set":8,"id":"e054a59a_ad0a2c60","line":105,"range":{"start_line":105,"start_character":40,"end_line":105,"end_character":45},"in_reply_to":"44d8454d_ebc19274","updated":"2026-02-27 23:34:18.000000000","message":"default is mode\u003d0o777 but isn\u0027t it too open? maybe we could include write permission to group as well.","commit_id":"7d083c10e48c9ec042a5c5d40e1184c6901451bb"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"c5442aeb992bb38fd237875e0eed60e3ef0e1c34","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        except FileNotFoundError:"},{"line_number":103,"context_line":"            # Directory doesn\u0027t exist, create it and retry"},{"line_number":104,"context_line":"            state_dir \u003d os.path.dirname(state_file)"},{"line_number":105,"context_line":"            os.makedirs(state_dir, mode\u003d0o755, exist_ok\u003dTrue)"},{"line_number":106,"context_line":"            with open(temp_file, \u0027w\u0027) as f:"},{"line_number":107,"context_line":"                json.dump(state, f)"},{"line_number":108,"context_line":"            os.rename(temp_file, state_file)"}],"source_content_type":"text/x-python","patch_set":8,"id":"f1a309b7_3b6fd482","line":105,"range":{"start_line":105,"start_character":40,"end_line":105,"end_character":45},"in_reply_to":"e054a59a_ad0a2c60","updated":"2026-03-25 14:58:59.000000000","message":"Marking as resolved. @rajatdhasmana, feel free to reopen if you need further clarification.","commit_id":"7d083c10e48c9ec042a5c5d40e1184c6901451bb"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5306f29089763743d4126f20b69461927f8a4283","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        def _umount_with_lock():"},{"line_number":213,"context_line":"            state \u003d self._get_state(mountpoint)"},{"line_number":214,"context_line":"            current_count \u003d state.get(\"attachment_count\", 0)"},{"line_number":215,"context_line":"            if current_count \u003d\u003d 0:"},{"line_number":216,"context_line":"                LOG.warning(_LW(\"Request to remove attachment \""},{"line_number":217,"context_line":"                                \"(%(vol_name)s, %(host)s) from \""},{"line_number":218,"context_line":"                                \"%(mountpoint)s, but attachment count is \""},{"line_number":219,"context_line":"                                \"already %(count)d.\"),"},{"line_number":220,"context_line":"                            {\u0027vol_name\u0027: vol_name, \u0027host\u0027: host,"},{"line_number":221,"context_line":"                             \u0027mountpoint\u0027: mountpoint,"},{"line_number":222,"context_line":"                             \u0027count\u0027: current_count})"},{"line_number":223,"context_line":"                return"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"            # Decrement attachment count"},{"line_number":226,"context_line":"            new_count \u003d current_count - 1"}],"source_content_type":"text/x-python","patch_set":8,"id":"bce568d6_1dd9e685","line":223,"range":{"start_line":215,"start_character":12,"end_line":223,"end_character":22},"updated":"2025-12-24 07:59:55.000000000","message":"what happens if the attachment_count is 0 but the mountpoint still exists? shouldn\u0027t we check for the existence of mount path as well?\n\n    mounted \u003d os.path.ismount(mountpoint)\n    if not mounted:\n        return\n    # Try to unmount","commit_id":"7d083c10e48c9ec042a5c5d40e1184c6901451bb"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"2e22c78e593872291311d7b34fc5a65cb0f48d09","unresolved":false,"context_lines":[{"line_number":212,"context_line":"        def _umount_with_lock():"},{"line_number":213,"context_line":"            state \u003d self._get_state(mountpoint)"},{"line_number":214,"context_line":"            current_count \u003d state.get(\"attachment_count\", 0)"},{"line_number":215,"context_line":"            if current_count \u003d\u003d 0:"},{"line_number":216,"context_line":"                LOG.warning(_LW(\"Request to remove attachment \""},{"line_number":217,"context_line":"                                \"(%(vol_name)s, %(host)s) from \""},{"line_number":218,"context_line":"                                \"%(mountpoint)s, but attachment count is \""},{"line_number":219,"context_line":"                                \"already %(count)d.\"),"},{"line_number":220,"context_line":"                            {\u0027vol_name\u0027: vol_name, \u0027host\u0027: host,"},{"line_number":221,"context_line":"                             \u0027mountpoint\u0027: mountpoint,"},{"line_number":222,"context_line":"                             \u0027count\u0027: current_count})"},{"line_number":223,"context_line":"                return"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"            # Decrement attachment count"},{"line_number":226,"context_line":"            new_count \u003d current_count - 1"}],"source_content_type":"text/x-python","patch_set":8,"id":"2ae130cd_b19637c8","line":223,"range":{"start_line":215,"start_character":12,"end_line":223,"end_character":22},"in_reply_to":"bce568d6_1dd9e685","updated":"2026-02-27 23:34:18.000000000","message":"Included your suggestion to the code. This should help self recoverying in case of a previous fail to unmount.","commit_id":"7d083c10e48c9ec042a5c5d40e1184c6901451bb"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"e6a66324471352029dbd1edfe1883998402c066c","unresolved":true,"context_lines":[{"line_number":149,"context_line":""},{"line_number":150,"context_line":"                # Create mountpoint directory if it doesn\u0027t exist"},{"line_number":151,"context_line":"                if not os.path.exists(mountpoint):"},{"line_number":152,"context_line":"                    os.makedirs(mountpoint, mode\u003d0o775, exist_ok\u003dTrue)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"                mount_cmd \u003d [\u0027mount\u0027, \u0027-t\u0027, fstype]"},{"line_number":155,"context_line":"                if options is not None:"}],"source_content_type":"text/x-python","patch_set":9,"id":"acccafed_202c603a","line":152,"range":{"start_line":152,"start_character":49,"end_line":152,"end_character":54},"updated":"2026-01-12 05:32:11.000000000","message":"Why did we raise the permission for group from RX to RWX?\nstill skeptical about the group and other permissions, we should follow principle of least privilege to make sure there aren\u0027t any security loopholes here.","commit_id":"5711fe377294d2b780dc6b23bcc88852aeacaeb8"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"c5442aeb992bb38fd237875e0eed60e3ef0e1c34","unresolved":true,"context_lines":[{"line_number":149,"context_line":""},{"line_number":150,"context_line":"                # Create mountpoint directory if it doesn\u0027t exist"},{"line_number":151,"context_line":"                if not os.path.exists(mountpoint):"},{"line_number":152,"context_line":"                    os.makedirs(mountpoint, mode\u003d0o775, exist_ok\u003dTrue)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"                mount_cmd \u003d [\u0027mount\u0027, \u0027-t\u0027, fstype]"},{"line_number":155,"context_line":"                if options is not None:"}],"source_content_type":"text/x-python","patch_set":9,"id":"bdaed74a_61d49065","line":152,"range":{"start_line":152,"start_character":49,"end_line":152,"end_character":54},"in_reply_to":"acccafed_202c603a","updated":"2026-03-25 14:58:59.000000000","message":"Same discussion as in comments for ln #105. The default is mode\u003d0o777, so I\u0027m only restricting write access to \"Others\" group with this param. Do you think we should restrict it even further, like dropping \"Others group\" access completelytely?","commit_id":"5711fe377294d2b780dc6b23bcc88852aeacaeb8"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"3c077ed6d7fa99efeca7fc611fdc469d11d0ff86","unresolved":false,"context_lines":[{"line_number":149,"context_line":""},{"line_number":150,"context_line":"                # Create mountpoint directory if it doesn\u0027t exist"},{"line_number":151,"context_line":"                if not os.path.exists(mountpoint):"},{"line_number":152,"context_line":"                    os.makedirs(mountpoint, mode\u003d0o775, exist_ok\u003dTrue)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"                mount_cmd \u003d [\u0027mount\u0027, \u0027-t\u0027, fstype]"},{"line_number":155,"context_line":"                if options is not None:"}],"source_content_type":"text/x-python","patch_set":9,"id":"36744fd3_0911007c","line":152,"range":{"start_line":152,"start_character":49,"end_line":152,"end_character":54},"in_reply_to":"bdaed74a_61d49065","updated":"2026-04-29 16:38:40.000000000","message":"set to mode\u003d0o755 as common for system accounts.","commit_id":"5711fe377294d2b780dc6b23bcc88852aeacaeb8"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"9990a053e1fc87d5638ef4841053b8f43f7fba70","unresolved":true,"context_lines":[{"line_number":180,"context_line":"            state \u003d self._get_state(mountpoint)"},{"line_number":181,"context_line":"            # Increment attachment count and add metadata"},{"line_number":182,"context_line":"            state[\"attachment_count\"] \u003d state.get(\"attachment_count\", 0) + 1"},{"line_number":183,"context_line":"            # Add metadata if not already present"},{"line_number":184,"context_line":"            if \"mountpoint\" not in state:"},{"line_number":185,"context_line":"                state[\"mountpoint\"] \u003d mountpoint"},{"line_number":186,"context_line":"            if \"export\" not in state:"}],"source_content_type":"text/x-python","patch_set":9,"id":"d73389c5_26a69772","line":183,"range":{"start_line":183,"start_character":0,"end_line":183,"end_character":12},"updated":"2026-02-16 16:05:37.000000000","message":"I don\u0027t think we ever read \"mountpoint\" or \"export\". Should these keys be removed?","commit_id":"5711fe377294d2b780dc6b23bcc88852aeacaeb8"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"2e22c78e593872291311d7b34fc5a65cb0f48d09","unresolved":false,"context_lines":[{"line_number":180,"context_line":"            state \u003d self._get_state(mountpoint)"},{"line_number":181,"context_line":"            # Increment attachment count and add metadata"},{"line_number":182,"context_line":"            state[\"attachment_count\"] \u003d state.get(\"attachment_count\", 0) + 1"},{"line_number":183,"context_line":"            # Add metadata if not already present"},{"line_number":184,"context_line":"            if \"mountpoint\" not in state:"},{"line_number":185,"context_line":"                state[\"mountpoint\"] \u003d mountpoint"},{"line_number":186,"context_line":"            if \"export\" not in state:"}],"source_content_type":"text/x-python","patch_set":9,"id":"f0bca2cd_90ebc6ac","line":183,"range":{"start_line":183,"start_character":0,"end_line":183,"end_character":12},"in_reply_to":"d73389c5_26a69772","updated":"2026-02-27 23:34:18.000000000","message":"Yeah this can be removed. These keys were usuful while testing but the code itself have no use of it. I\u0027m dropping both mountpoint and export keys.","commit_id":"5711fe377294d2b780dc6b23bcc88852aeacaeb8"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"bf6b8fb0cd4438bbd4e3df1548f13e437f6b026c","unresolved":true,"context_lines":[{"line_number":141,"context_line":"                   \u0027mountpoint\u0027: mountpoint, \u0027options\u0027: options, \u0027host\u0027: host})"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        @lockutils.synchronized(\u0027fs_mount_%s\u0027 % os.path.basename(mountpoint),"},{"line_number":144,"context_line":"                                external\u003dTrue)"},{"line_number":145,"context_line":"        def _mount_with_lock():"},{"line_number":146,"context_line":"            if not os.path.ismount(mountpoint):"},{"line_number":147,"context_line":"                LOG.debug(\u0027Mounting %(mountpoint)s\u0027,"}],"source_content_type":"text/x-python","patch_set":12,"id":"4b8817da_5232ba20","line":144,"range":{"start_line":144,"start_character":32,"end_line":144,"end_character":40},"updated":"2026-04-13 16:28:19.000000000","message":"This might be a bit of a stretch, but can we end up in a situation where we try to mount two different volumes at the same time, and these two volumes have a mountpoint with the same basename? Like `/media/pool1/volume1` and `/media/pool2/volume1`?\n\nIn this case, the lock for both volumes would be `fs_mount_volume1`.","commit_id":"18207c1f514a716df845699a7638879a6878d4d3"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"5cd9791ac902609d0ca0301dcb3a65a0747088aa","unresolved":true,"context_lines":[{"line_number":141,"context_line":"                   \u0027mountpoint\u0027: mountpoint, \u0027options\u0027: options, \u0027host\u0027: host})"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        @lockutils.synchronized(\u0027fs_mount_%s\u0027 % os.path.basename(mountpoint),"},{"line_number":144,"context_line":"                                external\u003dTrue)"},{"line_number":145,"context_line":"        def _mount_with_lock():"},{"line_number":146,"context_line":"            if not os.path.ismount(mountpoint):"},{"line_number":147,"context_line":"                LOG.debug(\u0027Mounting %(mountpoint)s\u0027,"}],"source_content_type":"text/x-python","patch_set":12,"id":"5cebb6fd_763f82a2","line":144,"range":{"start_line":144,"start_character":32,"end_line":144,"end_character":40},"in_reply_to":"4b8817da_5232ba20","updated":"2026-04-13 17:10:05.000000000","message":"If I understand this code correctly -- mountpoint here is where the share will be mounted locally, right?  I think _get_mount_path() in nfs.py would have already translated the share into a hashed value, which would mean those two wouldn\u0027t collide because basename will be different?","commit_id":"18207c1f514a716df845699a7638879a6878d4d3"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"20ed3f290911d87a07ef189083ee64a3d90ca4e2","unresolved":true,"context_lines":[{"line_number":141,"context_line":"                   \u0027mountpoint\u0027: mountpoint, \u0027options\u0027: options, \u0027host\u0027: host})"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        @lockutils.synchronized(\u0027fs_mount_%s\u0027 % os.path.basename(mountpoint),"},{"line_number":144,"context_line":"                                external\u003dTrue)"},{"line_number":145,"context_line":"        def _mount_with_lock():"},{"line_number":146,"context_line":"            if not os.path.ismount(mountpoint):"},{"line_number":147,"context_line":"                LOG.debug(\u0027Mounting %(mountpoint)s\u0027,"}],"source_content_type":"text/x-python","patch_set":12,"id":"71c71001_6f2d1999","line":144,"range":{"start_line":144,"start_character":32,"end_line":144,"end_character":40},"in_reply_to":"4b8817da_5232ba20","updated":"2026-04-13 17:02:09.000000000","message":"That is correct, but considering mountpoints from Cinder are always a hash value, we ended up deciding to remove the additional complexity to sanitize mountpoint names.\n\nThis points to the discussion: https://review.opendev.org/c/openstack/glance_store/+/967626/comment/9d1f0c21_085e8a10/","commit_id":"18207c1f514a716df845699a7638879a6878d4d3"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"3c077ed6d7fa99efeca7fc611fdc469d11d0ff86","unresolved":false,"context_lines":[{"line_number":141,"context_line":"                   \u0027mountpoint\u0027: mountpoint, \u0027options\u0027: options, \u0027host\u0027: host})"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        @lockutils.synchronized(\u0027fs_mount_%s\u0027 % os.path.basename(mountpoint),"},{"line_number":144,"context_line":"                                external\u003dTrue)"},{"line_number":145,"context_line":"        def _mount_with_lock():"},{"line_number":146,"context_line":"            if not os.path.ismount(mountpoint):"},{"line_number":147,"context_line":"                LOG.debug(\u0027Mounting %(mountpoint)s\u0027,"}],"source_content_type":"text/x-python","patch_set":12,"id":"9c996e28_29aed1d5","line":144,"range":{"start_line":144,"start_character":32,"end_line":144,"end_character":40},"in_reply_to":"5cebb6fd_763f82a2","updated":"2026-04-29 16:38:40.000000000","message":"Marking as resolved. Cyril, let me know if this should be discussed further.","commit_id":"18207c1f514a716df845699a7638879a6878d4d3"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"b1dd7192807c5f084bd42fe70a061dd87c5e8d04","unresolved":true,"context_lines":[{"line_number":95,"context_line":"        state_file \u003d self._get_state_file_path(mountpoint)"},{"line_number":96,"context_line":"        temp_file \u003d state_file + \u0027.tmp\u0027"},{"line_number":97,"context_line":"        try:"},{"line_number":98,"context_line":"            # Write state atomically using a temporary file"},{"line_number":99,"context_line":"            with open(temp_file, \u0027w\u0027) as f:"},{"line_number":100,"context_line":"                json.dump(state, f)"},{"line_number":101,"context_line":"            os.rename(temp_file, state_file)"}],"source_content_type":"text/x-python","patch_set":13,"id":"671af221_a345e75f","line":98,"updated":"2026-04-13 17:18:11.000000000","message":"Could just call\n\n os.makedirs(..., exist_ok\u003dTrue)\n\nhere to avoid duplicating lines 99 and 106.","commit_id":"b55bcec326b59cafa788cc32cf7c85718607810b"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"ba3b0ab02947e8dd719150b5b316bc6af7e6d843","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        state_file \u003d self._get_state_file_path(mountpoint)"},{"line_number":96,"context_line":"        temp_file \u003d state_file + \u0027.tmp\u0027"},{"line_number":97,"context_line":"        try:"},{"line_number":98,"context_line":"            # Write state atomically using a temporary file"},{"line_number":99,"context_line":"            with open(temp_file, \u0027w\u0027) as f:"},{"line_number":100,"context_line":"                json.dump(state, f)"},{"line_number":101,"context_line":"            os.rename(temp_file, state_file)"}],"source_content_type":"text/x-python","patch_set":13,"id":"8f49fdf5_c451f610","line":98,"in_reply_to":"671af221_a345e75f","updated":"2026-04-14 19:47:10.000000000","message":"Done","commit_id":"b55bcec326b59cafa788cc32cf7c85718607810b"}],"glance_store/tests/unit/common/test_fs_mount.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5306f29089763743d4126f20b69461927f8a4283","unresolved":true,"context_lines":[{"line_number":23,"context_line":"CONF \u003d cfg.CONF"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class HostMountStateTestCase(base.BaseTestCase):"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    def setUp(self):"},{"line_number":29,"context_line":"        super(HostMountStateTestCase, self).setUp()"}],"source_content_type":"text/x-python","patch_set":8,"id":"ec5200da_546ec6f5","line":26,"updated":"2025-12-24 07:59:55.000000000","message":"Few scenarios that i think should be tested, ignore if already covered with existing tests:\n\nMounting\n1. Mounting with path already mounted\n2. Mounting but mount command fails\n2.1 mounted\n2.2 not mounted\n3. mounting with attachment count 0\n4. mounting with attachment count \u003e\u003d 1\n5. mounting with mount command succeeds\n5.1 with attachment count 0\n5.2 with attachment count \u003e\u003d 1\n\nUnmounting\n1. Unmounting when already unmounted\n1.1 with attachment count 0\n1.2 with attachment count \u003e\u003d 1\n2. unmounting but umount operation fails\n2.1 with attachment count 0\n2.2 with attachment count \u003e\u003d 1\n3. unmounting succeeds\n3.1 with attachment count 0\n3.2 with attachment count \u003e\u003d 1\n\nOTHERS\n1. _get_state_file_path returning the right state path\n2. _get_state\n2.1 when state file doesn\u0027t exist\n2.2 fails to read to the state file\n2.3 success\n3. _save_state\n3.1 when state file doesn\u0027t exist\n3.2 fails to write to the state file","commit_id":"7d083c10e48c9ec042a5c5d40e1184c6901451bb"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"c5442aeb992bb38fd237875e0eed60e3ef0e1c34","unresolved":false,"context_lines":[{"line_number":23,"context_line":"CONF \u003d cfg.CONF"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class HostMountStateTestCase(base.BaseTestCase):"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    def setUp(self):"},{"line_number":29,"context_line":"        super(HostMountStateTestCase, self).setUp()"}],"source_content_type":"text/x-python","patch_set":8,"id":"406c863f_f27c5e7f","line":26,"in_reply_to":"ec5200da_546ec6f5","updated":"2026-03-25 14:58:59.000000000","message":"UTs included in the last patchset.","commit_id":"7d083c10e48c9ec042a5c5d40e1184c6901451bb"}],"releasenotes/notes/bug-967626-fix-race-conditions-mount-umount-operations-a2fb4054b9f8e8e2.yaml":[{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"bf6b8fb0cd4438bbd4e3df1548f13e437f6b026c","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    `Bug #967626 \u003chttps://bugs.launchpad.net/glance-store/+bug/967626\u003e`_:"},{"line_number":5,"context_line":"    Fixed race condition causing mount/umount operations to fail in"},{"line_number":6,"context_line":"    multi-worker Glance deployment with uWSGI."}],"source_content_type":"text/x-yaml","patch_set":12,"id":"423ef737_8faf4629","line":4,"range":{"start_line":4,"start_character":18,"end_line":4,"end_character":69},"updated":"2026-04-13 16:28:19.000000000","message":"This is a 404. Did you mean https://bugs.launchpad.net/glance/+bug/2115906 ?","commit_id":"18207c1f514a716df845699a7638879a6878d4d3"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"20ed3f290911d87a07ef189083ee64a3d90ca4e2","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    `Bug #967626 \u003chttps://bugs.launchpad.net/glance-store/+bug/967626\u003e`_:"},{"line_number":5,"context_line":"    Fixed race condition causing mount/umount operations to fail in"},{"line_number":6,"context_line":"    multi-worker Glance deployment with uWSGI."}],"source_content_type":"text/x-yaml","patch_set":12,"id":"7cc1f07a_a9f46fb7","line":4,"range":{"start_line":4,"start_character":18,"end_line":4,"end_character":69},"in_reply_to":"423ef737_8faf4629","updated":"2026-04-13 17:02:09.000000000","message":"Good catch! Will fix it.","commit_id":"18207c1f514a716df845699a7638879a6878d4d3"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"4c51a7e512191d6d73f69758160e668ed024e710","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    `Bug #967626 \u003chttps://bugs.launchpad.net/glance-store/+bug/967626\u003e`_:"},{"line_number":5,"context_line":"    Fixed race condition causing mount/umount operations to fail in"},{"line_number":6,"context_line":"    multi-worker Glance deployment with uWSGI."}],"source_content_type":"text/x-yaml","patch_set":12,"id":"9f74c2c9_c69c27f3","line":4,"range":{"start_line":4,"start_character":18,"end_line":4,"end_character":69},"in_reply_to":"7cc1f07a_a9f46fb7","updated":"2026-04-13 17:02:36.000000000","message":"Done","commit_id":"18207c1f514a716df845699a7638879a6878d4d3"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"06c4cba9858cffefea45c587fb0560574c7e776a","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    `Bug #967626 \u003chttps://bugs.launchpad.net/glance/+bug/2115906\u003e`_:"},{"line_number":5,"context_line":"    Fixed race condition causing mount/umount operations to fail in"},{"line_number":6,"context_line":"    multi-worker Glance deployment with uWSGI."}],"source_content_type":"text/x-yaml","patch_set":14,"id":"2800221a_49366de7","line":4,"range":{"start_line":4,"start_character":10,"end_line":4,"end_character":16},"updated":"2026-04-13 19:18:47.000000000","message":"Yes but it\u0027s still not the right bug number :p","commit_id":"1f9fade1447126abe103e70f29bfe26e5b9d2aad"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"c8a60db3f4f9e77df2ab549d3257c14978762d99","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    `Bug #967626 \u003chttps://bugs.launchpad.net/glance/+bug/2115906\u003e`_:"},{"line_number":5,"context_line":"    Fixed race condition causing mount/umount operations to fail in"},{"line_number":6,"context_line":"    multi-worker Glance deployment with uWSGI."}],"source_content_type":"text/x-yaml","patch_set":14,"id":"3c469664_21ece86e","line":4,"range":{"start_line":4,"start_character":10,"end_line":4,"end_character":16},"in_reply_to":"2800221a_49366de7","updated":"2026-04-13 19:23:20.000000000","message":"Sorry Cyril, my bad! was definitely trying to rush this, should have checked the whole message in reno.","commit_id":"1f9fade1447126abe103e70f29bfe26e5b9d2aad"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"6ff26020af6af22c335c5b76fc781fc5c7f5a58e","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    `Bug #967626 \u003chttps://bugs.launchpad.net/glance/+bug/2115906\u003e`_:"},{"line_number":5,"context_line":"    Fixed race condition causing mount/umount operations to fail in"},{"line_number":6,"context_line":"    multi-worker Glance deployment with uWSGI."}],"source_content_type":"text/x-yaml","patch_set":14,"id":"2308e7af_e5adb62c","line":4,"range":{"start_line":4,"start_character":10,"end_line":4,"end_character":16},"in_reply_to":"3c469664_21ece86e","updated":"2026-04-14 19:52:25.000000000","message":"I\u0027ve double checked the bug number and its launchpad link. Should be good now.","commit_id":"1f9fade1447126abe103e70f29bfe26e5b9d2aad"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"c038f5745842122b4f8e390ac63d6041c39647af","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":17,"id":"c1939bc6_fdc18b74","updated":"2026-04-14 20:10:39.000000000","message":"Should rename the file to match the bug number","commit_id":"5b5ee8b11ea07a52e34075c7daeb1db5face4558"},{"author":{"_account_id":30555,"name":"Fernando Ferraz","display_name":"Fernando Ferraz","email":"fesilva@redhat.com","username":"fernandoperches"},"change_message_id":"1f58f0c729508ee28dc57f2fc64090ada1462e96","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"bd0ebaca_0042b656","in_reply_to":"c1939bc6_fdc18b74","updated":"2026-04-14 20:16:01.000000000","message":"You\u0027re right, I\u0027ve renamed it to match the correct bug number.","commit_id":"5b5ee8b11ea07a52e34075c7daeb1db5face4558"}]}