Skip to content

Commit b79f4f9

Browse files
committed
move methods to vars, reduce size of dispatch
1 parent 6e6d629 commit b79f4f9

5 files changed

Lines changed: 108 additions & 141 deletions

File tree

src/blkcache/backend.py

Lines changed: 41 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -7,88 +7,37 @@
77
"""
88

99
import logging
10+
from functools import partial
1011
from pathlib import Path
11-
from typing import Dict, Any
1212

1313
# Import file detection
1414
from blkcache.file import detect
1515
from blkcache.file.device import DEFAULT_SECTOR_SIZE
1616

1717
log = logging.getLogger(__name__)
1818

19-
20-
class HandleManager:
21-
"""Manages file handles and their lifecycle for nbdkit backend."""
22-
23-
def __init__(self):
24-
self._handles: Dict[int, Any] = {} # handle_id -> File instance
25-
self._next_handle = 1
26-
self._files = [] # Keep references to context managers
27-
28-
def open_file(self, path: Path, mode: str) -> int:
29-
"""Open a file and return a handle ID."""
30-
file_cls = detect(path)
31-
file_instance = file_cls(path, mode)
32-
33-
# Enter the context manager and keep reference
34-
opened_file = file_instance.__enter__()
35-
self._files.append((file_instance, opened_file))
36-
37-
# Assign handle and store
38-
handle = self._next_handle
39-
self._handles[handle] = opened_file
40-
self._next_handle += 1
41-
42-
log.debug("Opened file %s as handle %d", path, handle)
43-
return handle
44-
45-
def get_file(self, handle: int):
46-
"""Get the file instance for a handle."""
47-
if handle not in self._handles:
48-
raise ValueError(f"Invalid handle: {handle}")
49-
return self._handles[handle]
50-
51-
def close_file(self, handle: int) -> None:
52-
"""Close a specific file handle."""
53-
if handle in self._handles:
54-
file_instance = self._handles[handle]
55-
# Find and close the corresponding context manager
56-
for i, (ctx_mgr, opened_file) in enumerate(self._files):
57-
if opened_file is file_instance:
58-
try:
59-
ctx_mgr.__exit__(None, None, None)
60-
except Exception as e:
61-
log.warning("Error closing file handle %d: %s", handle, e)
62-
self._files.pop(i)
63-
break
64-
65-
del self._handles[handle]
66-
log.debug("Closed handle %d", handle)
67-
68-
def close_all(self) -> None:
69-
"""Close all open files."""
70-
for ctx_mgr, _ in self._files:
71-
try:
72-
ctx_mgr.__exit__(None, None, None)
73-
except Exception as e:
74-
log.warning("Error during cleanup: %s", e)
75-
76-
self._handles.clear()
77-
self._files.clear()
78-
log.debug("Closed all handles")
79-
80-
8119
# Global state
8220
DEV: Path | None = None
8321
CACHE: Path | None = None
8422
SECTOR_SIZE = DEFAULT_SECTOR_SIZE
8523
METADATA = {}
8624

87-
# Handle manager instance
88-
HANDLE_MANAGER = HandleManager()
25+
# Simple dispatch table: handle -> (file_instance, context_generator)
26+
TABLE = {}
27+
28+
29+
def lookup(attr: str, handle: int, table=TABLE):
30+
"""Generic attribute lookup for dispatch."""
31+
obj, _ = table[handle]
32+
return getattr(obj, attr)
8933

9034

91-
# These functions are now handled by the BlockCache class
35+
def open_file_context(path: Path, mode: str):
36+
"""Generator to manage file lifecycle."""
37+
file_cls = detect(path)
38+
file_instance = file_cls(path, mode)
39+
with file_instance as f:
40+
yield f
9241

9342

9443
def config(key: str, val: str) -> None:
@@ -126,72 +75,45 @@ def config_complete() -> None:
12675
def open(_readonly: bool) -> int:
12776
"""Opens device and returns handle ID."""
12877
mode = "rb" if _readonly else "r+b"
129-
return HANDLE_MANAGER.open_file(DEV, mode)
78+
handle = len(TABLE) + 1
79+
ctx = open_file_context(DEV, mode)
80+
obj = next(ctx)
81+
TABLE[handle] = (obj, ctx)
82+
log.debug("Opened file %s as handle %d", DEV, handle)
83+
return handle
13084

13185

13286
def get_size(h: int) -> int:
13387
"""Get file size."""
134-
file_instance = HANDLE_MANAGER.get_file(h)
135-
return file_instance.size()
88+
obj, _ = TABLE[h]
89+
return obj.size()
13690

13791

13892
def pread(h: int, count: int, offset: int) -> bytes:
13993
"""Read data at offset."""
140-
file_instance = HANDLE_MANAGER.get_file(h)
141-
return file_instance.pread(count, offset)
94+
obj, _ = TABLE[h]
95+
return obj.pread(count, offset)
14296

14397

14498
def close(h: int) -> None:
14599
"""Close file handle."""
146100
log.debug("Backend close() called for handle %d", h)
147-
HANDLE_MANAGER.close_file(h)
101+
if h in TABLE:
102+
obj, ctx = TABLE[h]
103+
try:
104+
next(ctx) # Advance generator to trigger cleanup
105+
except StopIteration:
106+
pass
107+
del TABLE[h]
148108
log.debug("Backend close() completed")
149109

150110

151-
# Optional capability functions - use duck typing
152-
def can_write(h: int) -> bool:
153-
"""Check if file supports writing."""
154-
file_instance = HANDLE_MANAGER.get_file(h)
155-
return "w" in file_instance.mode or "a" in file_instance.mode or "+" in file_instance.mode
156-
157-
158-
def can_flush(h: int) -> bool:
159-
"""Check if file supports flushing."""
160-
file_instance = HANDLE_MANAGER.get_file(h)
161-
return hasattr(file_instance, "flush")
162-
163-
164-
def can_trim(h: int) -> bool:
165-
"""Check if file supports trim operations."""
166-
file_instance = HANDLE_MANAGER.get_file(h)
167-
return hasattr(file_instance, "trim")
168-
169-
170-
def can_zero(h: int) -> bool:
171-
"""Check if file supports zero operations."""
172-
file_instance = HANDLE_MANAGER.get_file(h)
173-
return hasattr(file_instance, "zero")
174-
175-
176-
def can_fast_zero(h: int) -> bool:
177-
"""Check if file supports fast zero operations."""
178-
file_instance = HANDLE_MANAGER.get_file(h)
179-
return hasattr(file_instance, "fast_zero")
180-
181-
182-
def can_extents(h: int) -> bool:
183-
"""Check if file supports extent operations."""
184-
file_instance = HANDLE_MANAGER.get_file(h)
185-
return hasattr(file_instance, "extents")
186-
187-
188-
def is_rotational(h: int) -> bool:
189-
"""Check if underlying storage is rotational."""
190-
file_instance = HANDLE_MANAGER.get_file(h)
191-
return hasattr(file_instance, "is_rotational") and file_instance.is_rotational()
192-
193-
194-
def can_multi_conn(h: int) -> bool:
195-
"""Check if file supports multiple connections."""
196-
# For now, return False for safety
197-
return False
111+
# Optional capability functions - use partial dispatch
112+
can_write = partial(lookup, "can_write")
113+
can_flush = partial(lookup, "can_flush")
114+
can_trim = partial(lookup, "can_trim")
115+
can_zero = partial(lookup, "can_zero")
116+
can_fast_zero = partial(lookup, "can_fast_zero")
117+
can_extents = partial(lookup, "can_extents")
118+
is_rotational = partial(lookup, "is_rotational")
119+
can_multi_conn = partial(lookup, "can_multi_conn")

src/blkcache/file/base.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import hashlib
2+
from functools import lru_cache
23
from pathlib import Path
34

45

@@ -9,20 +10,42 @@ def __init__(self, path: Path | str, mode: str):
910
self.path = Path(path)
1011
self.mode = mode
1112
self._f = None
13+
self._dependencies = []
14+
15+
# nbdkit capability attributes - safe defaults
16+
self.can_write = "w" in mode or "+" in mode or "a" in mode
17+
self.can_flush = True
18+
self.can_trim = False
19+
self.can_zero = False
20+
self.can_fast_zero = False
21+
self.can_extents = False
22+
self.is_rotational = False
23+
self.can_multi_conn = False
1224

1325
@staticmethod
1426
def check(path: Path) -> bool:
1527
"""Check if this class can handle the given path."""
1628
return path.is_file() or (path.exists() and not path.is_dir())
1729

30+
def depends(self, *files):
31+
"""Register file dependencies for cascading cleanup."""
32+
self._dependencies.extend(files)
33+
return self
34+
1835
def __enter__(self):
36+
# Open dependencies first (bottom-up)
37+
for dep in self._dependencies:
38+
dep.__enter__()
1939
self._f = self.path.open(self.mode)
2040
return self
2141

2242
def __exit__(self, exc_type, exc_val, exc_tb):
2343
if self._f:
2444
self._f.close()
2545
self._f = None
46+
# Close dependencies last (top-down)
47+
for dep in reversed(self._dependencies):
48+
dep.__exit__(exc_type, exc_val, exc_tb)
2649

2750
def __getattr__(self, name):
2851
"""Delegate unknown attributes to the underlying file object."""
@@ -63,6 +86,8 @@ def size(self) -> int:
6386
finally:
6487
self._f.seek(current)
6588

89+
@property
90+
@lru_cache(maxsize=1)
6691
def sector_size(self) -> int:
6792
"""Get sector size of the underlying storage device."""
6893
try:
Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
from .base import File
1111

1212

13-
class CacheFile(File):
13+
class CachedFile(File):
1414
"""Passthrough cache that wraps another File instance."""
1515

1616
def __init__(self, backing_file: File, cache_file: File):
@@ -49,9 +49,10 @@ def size(self) -> int:
4949
"""Get size from backing file."""
5050
return self.backing_file.size()
5151

52+
@property
5253
def sector_size(self) -> int:
5354
"""Get sector size from backing file."""
54-
return self.backing_file.sector_size()
55+
return self.backing_file.sector_size
5556

5657
def pread(self, count: int, offset: int) -> bytes:
5758
"""Read with cache - try cache first, then backing file."""

src/blkcache/file/device.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import fcntl
99
import struct
10+
from functools import lru_cache
1011
from pathlib import Path
1112

1213
from .base import File
@@ -24,6 +25,8 @@ class Device(File):
2425

2526
def __init__(self, path: Path | str, mode: str = "rb"):
2627
super().__init__(path, mode)
28+
# Update capability based on device type
29+
self.is_rotational = self._check_rotational()
2730

2831
@staticmethod
2932
def check(path: Path) -> bool:
@@ -48,6 +51,8 @@ def device_size(self) -> int:
4851
# Fall back to file size
4952
return self.size()
5053

54+
@property
55+
@lru_cache(maxsize=1)
5156
def sector_size(self) -> int:
5257
"""Get device sector size using ioctl."""
5358
try:
@@ -56,7 +61,7 @@ def sector_size(self) -> int:
5661
except (OSError, IOError):
5762
return DEFAULT_SECTOR_SIZE
5863

59-
def is_rotational(self) -> bool:
64+
def _check_rotational(self) -> bool:
6065
"""Check if device uses spinning media (HDD) vs flash (SSD)."""
6166
try:
6267
# Check sys path for rotational status

src/blkcache/file/removable.py

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import struct
1111
import threading
1212
import time
13+
from functools import lru_cache
1314
from pathlib import Path
1415

1516
from .device import Device, BLKSSZGET, DEFAULT_SECTOR_SIZE
@@ -24,6 +25,34 @@
2425
class Removable(Device):
2526
"""Removable device with media change detection."""
2627

28+
def __init__(self, path: Path | str, mode: str = "rb"):
29+
super().__init__(path, mode)
30+
# Optical drives and floppy disks are rotational
31+
self.is_rotational = self._is_optical() or self._is_floppy()
32+
33+
def _is_optical(self) -> bool:
34+
"""Check if this is an optical drive (CD/DVD)."""
35+
device_name = self.path.name
36+
return device_name.startswith(("sr", "scd")) or "cdrom" in str(self.path)
37+
38+
def _is_floppy(self) -> bool:
39+
"""Check if this is a floppy disk drive."""
40+
device_name = self.path.name
41+
# fd0, fd1 for traditional floppies, sd* devices need more checking
42+
if device_name.startswith("fd"):
43+
return True
44+
45+
# Check for USB floppies via sysfs
46+
try:
47+
model_path = Path(f"/sys/block/{device_name}/device/model")
48+
if model_path.exists():
49+
model = model_path.read_text().strip().lower()
50+
return "floppy" in model or "fd" in model
51+
except Exception:
52+
pass
53+
54+
return False
55+
2756
@staticmethod
2857
def check(path: Path) -> bool:
2958
"""Check if this is a removable block device."""
@@ -38,9 +67,11 @@ def check(path: Path) -> bool:
3867
except (OSError, IOError):
3968
pass
4069

41-
# Fallback: optical drive naming patterns
42-
return "sr" in str(path) or "cd" in str(path)
70+
p = str(path)
71+
return p.startswith("/dev/") and ("sr" in p or "cd" in p)
4372

73+
@property
74+
@lru_cache(maxsize=1)
4475
def sector_size(self) -> int:
4576
"""Get sector size with CDROM-specific ioctl support."""
4677
try:
@@ -56,23 +87,6 @@ def sector_size(self) -> int:
5687
return DEFAULT_CDROM_SECTOR_SIZE
5788
return DEFAULT_SECTOR_SIZE
5889

59-
def is_rotational(self) -> bool:
60-
"""Removable devices are typically rotational (CD/DVD)."""
61-
try:
62-
# Check sys path first
63-
rotational_path = Path(f"/sys/block/{self.path.name}/queue/rotational")
64-
if rotational_path.exists():
65-
return rotational_path.read_text().strip() == "1"
66-
except Exception:
67-
pass
68-
69-
# Heuristic: CDs and DVDs are rotational
70-
if "sr" in str(self.path) or "cd" in str(self.path):
71-
return True
72-
73-
# Default for other removable media
74-
return False
75-
7690
def watch_for_changes(self, stop_event: threading.Event, callback=None, logger=None) -> None:
7791
"""
7892
Monitor device for media changes.

0 commit comments

Comments
 (0)