block_device: remove cmp_mount_order
This comparator is buggy, and orders differently on python 3.13
$ python3.14 ./test.py
[('/', 'fake_name'), ('/var/log', 'fake_log'), ('/boot', 'fake_boot'), ('/var', 'fake_name')]
$ python3.10 ./test.py
[('/', 'fake_name'), ('/var', 'fake_name'), ('/var/log', 'fake_log'), ('/boot', 'fake_boot')]
It is saying that if it's not equal, or doesn't start with the same
prefix, it is bigger. Hence "/boot > /var" but also "/var > /boot"
which confuses things.
I really think what we want here is to ensure that parents mount
before children. So string comparision is fine? "/var < /var/log <
/var/log/foo"?
Replace it with that
Change-Id: Ieac277c90b13bdb8b2c651b17fa4617ac4c1b845
Signed-off-by: Ian Wienand <iwienand@redhat.com>
This commit is contained in:
committed by
Clark Boylan
parent
12121be8e9
commit
b97b424aa4
@@ -12,7 +12,6 @@
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import functools
|
||||
import logging
|
||||
import os
|
||||
import time
|
||||
@@ -137,30 +136,6 @@ class MountPointNode(NodeBase):
|
||||
self.umount()
|
||||
|
||||
|
||||
def cmp_mount_order(this, other):
|
||||
"""Sort comparision function for mount-point sorting
|
||||
|
||||
See if ``this`` comes before ``other`` in mount-order list. In
|
||||
words: if the other mount-point has us as it's parent, we come
|
||||
before it (are less than it). e.g. ``/var < /var/log <
|
||||
/var/log/foo``
|
||||
|
||||
:param this: tuple of mount_point, node name
|
||||
:param other: tuple of mount_point, node name
|
||||
:returns int: cmp value
|
||||
|
||||
"""
|
||||
# sort is only based on the mount_point.
|
||||
this, _ = this
|
||||
other, _ = other
|
||||
if this == other:
|
||||
return 0
|
||||
if other.startswith(this):
|
||||
return -1
|
||||
else:
|
||||
return 1
|
||||
|
||||
|
||||
class Mount(PluginBase):
|
||||
def __init__(self, config, defaults, state):
|
||||
super(Mount, self).__init__()
|
||||
@@ -185,7 +160,10 @@ class Mount(PluginBase):
|
||||
"Mount point [%s] specified more than once"
|
||||
% self.node.mount_point)
|
||||
sorted_mount_points.append((self.node.mount_point, self.node.name))
|
||||
sorted_mount_points.sort(key=functools.cmp_to_key(cmp_mount_order))
|
||||
# Sort by mount point so hierarchy is in shortest to longest, such
|
||||
# that parents mount before children; e.g.
|
||||
# /var < /var/log
|
||||
sorted_mount_points.sort(key=lambda x: x[0])
|
||||
# Save the state if it's new (otherwise this is idempotent update)
|
||||
state['sorted_mount_points'] = sorted_mount_points
|
||||
logger.debug("Ordered mounts now: %s", sorted_mount_points)
|
||||
|
||||
@@ -10,7 +10,6 @@
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import functools
|
||||
import logging
|
||||
import os
|
||||
from unittest import mock
|
||||
@@ -20,37 +19,11 @@ import diskimage_builder.block_device.tests.test_config as tc
|
||||
from diskimage_builder.block_device.config import config_tree_to_graph
|
||||
from diskimage_builder.block_device.config import create_graph
|
||||
from diskimage_builder.block_device.level2.mkfs import FilesystemNode
|
||||
from diskimage_builder.block_device.level3.mount import cmp_mount_order
|
||||
from diskimage_builder.block_device.level3.mount import MountPointNode
|
||||
from diskimage_builder.block_device.tests.test_base import TestBase
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class TestMountComparator(TestBase):
|
||||
|
||||
def test_mount_comparator(self):
|
||||
# This tests cmp_mount_order to ensure it sorts in the
|
||||
# expected order. The comparator takes a tuple of
|
||||
# (mount_point, node_name) but we can ignore the name
|
||||
partitions = [
|
||||
('/var/log', 'fake_log'),
|
||||
('/boot', 'fake_boot'),
|
||||
('/', 'fake_name'),
|
||||
('/var', 'fake_name')]
|
||||
partitions.sort(key=functools.cmp_to_key(cmp_mount_order))
|
||||
|
||||
res = list(x[0] for x in partitions)
|
||||
|
||||
# "/" must be first
|
||||
self.assertEqual(res[0], '/')
|
||||
|
||||
# /var before /var/log
|
||||
var_pos = res.index('/var')
|
||||
var_log_pos = res.index('/var/log')
|
||||
self.assertGreater(var_log_pos, var_pos)
|
||||
|
||||
|
||||
class TestMountOrder(tc.TestGraphGeneration):
|
||||
|
||||
def _exec_sudo_log(*args, **kwargs):
|
||||
|
||||
Reference in New Issue
Block a user