Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions fabrics.c
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,8 @@ int fabrics_disconnect(const char *desc, int argc, char **argv)
}

/* disconnect-all policy: should controller @c be torn down? */
static bool disconnect_all_match(libnvme_ctrl_t c, const char *transport,
static bool disconnect_all_match(struct libnvme_global_ctx *ctx,
libnvme_ctrl_t c, const char *transport,
const char *owner, bool force)
{
if (transport && strcmp(transport, libnvme_ctrl_get_transport(c)))
Expand All @@ -911,7 +912,12 @@ static bool disconnect_all_match(libnvme_ctrl_t c, const char *transport,
return false;
if (force)
return true;
return libnvmf_registry_attr_equal(libnvme_ctrl_get_name(c),

/*
* attr_equal() returns 0 only on an exact match; a read error (<0)
* compares as "not a match", so we never disconnect on error.
*/
return libnvmf_registry_attr_equal(ctx, libnvme_ctrl_get_name(c),
"owner", owner) == 0;
}

Expand Down Expand Up @@ -997,7 +1003,7 @@ int fabrics_disconnect_all(const char *desc, int argc, char **argv)
libnvme_for_each_host(ctx, h) {
libnvme_for_each_subsystem(h, s) {
libnvme_subsystem_for_each_ctrl(s, c) {
if (!disconnect_all_match(c, cfg.transport,
if (!disconnect_all_match(ctx, c, cfg.transport,
cfg.owner, cfg.force))
continue;
if (libnvmf_disconnect_ctrl(c))
Expand Down
51 changes: 32 additions & 19 deletions libnvme/libnvme/nvme.i
Original file line number Diff line number Diff line change
Expand Up @@ -504,11 +504,12 @@ PyObject *nbft_get(struct libnvme_global_ctx *ctx, const char *filename)
libnvmf_free_nbft(ctx, nbft);
return output;
}
void registry_update(const char *device, const char *attr, const char *value)
void registry_update(struct libnvme_global_ctx *ctx,
const char *device, const char *attr, const char *value)
{
int ret;

ret = libnvmf_registry_update(device, attr, value);
ret = libnvmf_registry_update(ctx, device, attr, value);
if (ret == -EINVAL)
PyErr_SetString(PyExc_ValueError,
"invalid device or attribute name");
Expand All @@ -517,11 +518,11 @@ void registry_update(const char *device, const char *attr, const char *value)
"registry_update failed: %s", strerror(-ret));
}

void registry_delete(const char *device)
void registry_delete(struct libnvme_global_ctx *ctx, const char *device)
{
int ret;

ret = libnvmf_registry_delete(device);
ret = libnvmf_registry_delete(ctx, device);
if (ret == -EINVAL)
PyErr_SetString(PyExc_ValueError, "invalid device name");
else if (ret == -ENOENT)
Expand All @@ -533,13 +534,14 @@ void registry_delete(const char *device)
device, strerror(-ret));
}

PyObject *registry_retrieve(const char *device, const char *attr)
PyObject *registry_retrieve(struct libnvme_global_ctx *ctx,
const char *device, const char *attr)
{
char *value = NULL;
PyObject *str;
int ret;

ret = libnvmf_registry_retrieve(device, attr, &value);
ret = libnvmf_registry_retrieve(ctx, device, attr, &value);
if (ret == -ENOENT)
Py_RETURN_NONE;
if (ret == -EINVAL) {
Expand Down Expand Up @@ -567,14 +569,14 @@ static void _registry_collect_device(const char *device, void *user_data)
}
}

PyObject *registry_devices(void)
PyObject *registry_devices(struct libnvme_global_ctx *ctx)
{
PyObject *list = PyList_New(0);
int ret;

if (!list)
return NULL;
ret = libnvmf_registry_device_for_each(_registry_collect_device, list);
ret = libnvmf_registry_device_for_each(ctx, _registry_collect_device, list);
if (ret < 0) {
Py_DECREF(list);
PyErr_Format(PyExc_OSError,
Expand All @@ -599,15 +601,16 @@ static void _registry_collect_attr(const char *attr, const char *value,
}
}

PyObject *registry_device_attrs(const char *device)
PyObject *registry_device_attrs(struct libnvme_global_ctx *ctx,
const char *device)
{
PyObject *dict = PyDict_New();
int ret;

if (!dict)
return NULL;
ret = libnvmf_registry_attr_for_each(device, _registry_collect_attr,
dict);
ret = libnvmf_registry_attr_for_each(ctx, device,
_registry_collect_attr, dict);
if (ret == -EINVAL) {
Py_DECREF(dict);
PyErr_SetString(PyExc_ValueError, "invalid device name");
Expand Down Expand Up @@ -675,19 +678,21 @@ from libnvme._exceptions import (
NotConnectedError,
)

def registry_entries():
def registry_entries(ctx):
"""Yield (device, attrs) for each live registry entry.

Wraps the C registry iterators: registry_devices() applies the /dev
stale-entry check and registry_device_attrs() reads each entry's
attributes -- all file access happens in libnvme.

ctx: nvme.GlobalCtx instance

Yields:
tuple: (device, attrs) where device is the kernel device name
(e.g. 'nvme3') and attrs is a dict of attribute name to value.
"""
for device in registry_devices():
yield device, registry_device_attrs(device)
for device in registry_devices(ctx):
yield device, registry_device_attrs(ctx, device)
%}

/*############################################################################*/
Expand Down Expand Up @@ -964,27 +969,35 @@ PyObject *nbft_get(struct libnvme_global_ctx *ctx, const char *filename);
$action
if (PyErr_Occurred()) SWIG_fail;
}
void registry_update(const char *device, const char *attr, const char *value);
void registry_update(struct libnvme_global_ctx *ctx,
const char *device, const char *attr, const char *value);

%exception registry_delete {
$action
if (PyErr_Occurred()) SWIG_fail;
}
void registry_delete(const char *device);
void registry_delete(struct libnvme_global_ctx *ctx,
const char *device);

%exception registry_retrieve {
$action
if (PyErr_Occurred()) SWIG_fail;
}
PyObject *registry_retrieve(const char *device, const char *attr);
PyObject *registry_retrieve(struct libnvme_global_ctx *ctx,
const char *device, const char *attr);

%exception registry_devices {
$action
if (PyErr_Occurred()) SWIG_fail;
}
PyObject *registry_devices(void);
PyObject *registry_devices(struct libnvme_global_ctx *ctx);

%exception registry_device_attrs {
$action
if (PyErr_Occurred()) SWIG_fail;
}
PyObject *registry_device_attrs(const char *device);
PyObject *registry_device_attrs(struct libnvme_global_ctx *ctx,
const char *device);

%rename(_libnvme_first_host) libnvme_first_host;
%rename(_libnvme_next_host) libnvme_next_host;
Expand Down
97 changes: 66 additions & 31 deletions libnvme/libnvme/tests/test-registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,16 @@

# dir='/tmp' is required: libnvme confines NVME_REGISTRY_DIR to /tmp, so the
# test directory must live there (not under an arbitrary $TMPDIR).
_tmpdir = tempfile.mkdtemp(prefix='nvme-registry-test-', dir='/tmp')
os.environ['NVME_REGISTRY_DIR'] = _tmpdir
#
# Reuse an inherited /tmp directory instead of always creating a new one: with
# the spawn/forkserver start methods (the default on Linux as of Python 3.14)
# each child re-imports this module. The child inherits NVME_REGISTRY_DIR from
# the parent, so it must reuse that path rather than create a second, unrelated
# registry directory it would then write into alone.
_tmpdir = os.environ.get('NVME_REGISTRY_DIR', '')
if not _tmpdir.startswith('/tmp/'):
_tmpdir = tempfile.mkdtemp(prefix='nvme-registry-test-', dir='/tmp')
os.environ['NVME_REGISTRY_DIR'] = _tmpdir

from libnvme import nvme # noqa: E402 (import after env var set intentionally)

Expand All @@ -35,51 +43,64 @@ def _teardown_tmpdir():


class TestRegistryUpdate(unittest.TestCase):
def setUp(self):
self.ctx = nvme.GlobalCtx()

def tearDown(self):
nvme.registry_delete('nvme5')
nvme.registry_delete(self.ctx, 'nvme5')
self.ctx = None

def test_update_creates_entry(self):
nvme.registry_update('nvme5', 'owner', 'stas')
value = nvme.registry_retrieve('nvme5', 'owner')
nvme.registry_update(self.ctx, 'nvme5', 'owner', 'stas')
value = nvme.registry_retrieve(self.ctx, 'nvme5', 'owner')
self.assertEqual(value, 'stas')

def test_update_steals_ownership(self):
nvme.registry_update('nvme5', 'owner', 'nbft')
nvme.registry_update('nvme5', 'owner', 'stas')
value = nvme.registry_retrieve('nvme5', 'owner')
nvme.registry_update(self.ctx, 'nvme5', 'owner', 'nbft')
nvme.registry_update(self.ctx, 'nvme5', 'owner', 'stas')
value = nvme.registry_retrieve(self.ctx, 'nvme5', 'owner')
self.assertEqual(value, 'stas')

def test_update_multiple_attrs(self):
nvme.registry_update('nvme5', 'owner', 'stas')
nvme.registry_update('nvme5', 'extra', 'hello')
self.assertEqual(nvme.registry_retrieve('nvme5', 'owner'), 'stas')
self.assertEqual(nvme.registry_retrieve('nvme5', 'extra'), 'hello')
nvme.registry_update(self.ctx, 'nvme5', 'owner', 'stas')
nvme.registry_update(self.ctx, 'nvme5', 'extra', 'hello')
self.assertEqual(nvme.registry_retrieve(self.ctx, 'nvme5', 'owner'), 'stas')
self.assertEqual(nvme.registry_retrieve(self.ctx, 'nvme5', 'extra'), 'hello')


class TestRegistryRetrieve(unittest.TestCase):
def setUp(self):
self.ctx = nvme.GlobalCtx()

def tearDown(self):
self.ctx = None

def test_retrieve_unregistered_returns_none(self):
value = nvme.registry_retrieve('nvme99', 'owner')
value = nvme.registry_retrieve(self.ctx, 'nvme99', 'owner')
self.assertIsNone(value)

def test_retrieve_missing_attr_returns_none(self):
nvme.registry_update('nvme6', 'owner', 'stas')
value = nvme.registry_retrieve('nvme6', 'nosuchattr')
nvme.registry_delete('nvme6')
nvme.registry_update(self.ctx, 'nvme6', 'owner', 'stas')
value = nvme.registry_retrieve(self.ctx, 'nvme6', 'nosuchattr')
nvme.registry_delete(self.ctx, 'nvme6')
self.assertIsNone(value)


class TestRegistryDelete(unittest.TestCase):
def setUp(self):
self.ctx = nvme.GlobalCtx()

def tearDown(self):
self.ctx = None

def test_delete_removes_entry(self):
nvme.registry_update('nvme7', 'owner', 'stas')
nvme.registry_delete('nvme7')
self.assertIsNone(nvme.registry_retrieve('nvme7', 'owner'))
nvme.registry_update(self.ctx, 'nvme7', 'owner', 'stas')
nvme.registry_delete(self.ctx, 'nvme7')
self.assertIsNone(nvme.registry_retrieve(self.ctx, 'nvme7', 'owner'))

def test_delete_nonexistent_raises(self):
with self.assertRaises(FileNotFoundError):
nvme.registry_delete('nvme99')
nvme.registry_delete(self.ctx, 'nvme99')


class TestRegistryEntries(unittest.TestCase):
Expand All @@ -88,40 +109,54 @@ class TestRegistryEntries(unittest.TestCase):
an iterable."""

def setUp(self):
nvme.registry_update('nvme1', 'owner', 'stas')
nvme.registry_update('nvme2', 'owner', 'nbft')
self.ctx = nvme.GlobalCtx()
nvme.registry_update(self.ctx, 'nvme1', 'owner', 'stas')
nvme.registry_update(self.ctx, 'nvme2', 'owner', 'nbft')

def tearDown(self):
nvme.registry_delete('nvme1')
nvme.registry_delete('nvme2')
nvme.registry_delete(self.ctx, 'nvme1')
nvme.registry_delete(self.ctx, 'nvme2')
self.ctx = None

def test_entries_returns_iterable(self):
entries = list(nvme.registry_entries())
entries = list(nvme.registry_entries(self.ctx))
# All entries are stale (no /dev/nvme* in test environment) — list is empty.
self.assertIsInstance(entries, list)

def test_entries_skips_stale(self):
for device, attrs in nvme.registry_entries():
for device, attrs in nvme.registry_entries(self.ctx):
self.assertTrue(os.path.exists('/dev/' + device))


def _writer(device, owner, iterations):
"""Child process: repeatedly update the owner attribute."""
"""Child process: repeatedly update the owner attribute.

Each process creates its own GlobalCtx. A libnvme context must not be
shared across a process boundary: passing it as a Process argument is not
picklable under the spawn/forkserver start methods, and even under fork a
context is not designed to be used concurrently from two processes.
"""
ctx = nvme.GlobalCtx()
for _ in range(iterations):
nvme.registry_update(device, 'owner', owner)
nvme.registry_update(ctx, device, 'owner', owner)


class TestRegistryParallelWrites(unittest.TestCase):
"""Verify that concurrent writes from multiple processes do not corrupt
the registry. The atomic tmp->rename write protocol must ensure the
final value is always one of the two written strings."""
def setUp(self):
self.ctx = nvme.GlobalCtx()

def tearDown(self):
self.ctx = None

@unittest.skipIf(_under_valgrind, "skipped under valgrind — covered by C test")
def test_parallel_writes_no_corruption(self):
nprocs = 10
owners = [f'proc{i}' for i in range(nprocs)]

nvme.registry_update('nvme10', 'owner', 'parent')
nvme.registry_update(self.ctx, 'nvme10', 'owner', 'parent')

procs = [multiprocessing.Process(target=_writer, args=('nvme10', owner, 200))
for owner in owners]
Expand All @@ -130,8 +165,8 @@ def test_parallel_writes_no_corruption(self):
for p in procs:
p.join()

value = nvme.registry_retrieve('nvme10', 'owner')
nvme.registry_delete('nvme10')
value = nvme.registry_retrieve(self.ctx, 'nvme10', 'owner')
nvme.registry_delete(self.ctx, 'nvme10')

self.assertIn(value, owners, f'corrupted value: {value!r}')

Expand Down
5 changes: 3 additions & 2 deletions libnvme/src/nvme/fabrics.c
Original file line number Diff line number Diff line change
Expand Up @@ -985,9 +985,10 @@ static void registry_update_on_connect(struct libnvme_global_ctx *ctx,
int ret;

if (ctx->owner)
ret = libnvmf_registry_create_instance(instance, ctx->owner);
ret = libnvmf_registry_create_instance(ctx, instance,
ctx->owner);
else
ret = libnvmf_registry_delete_instance(instance);
ret = libnvmf_registry_delete_instance(ctx, instance);
if (ret)
libnvme_msg(ctx, LIBNVME_LOG_WARN,
"nvme%d: registry update failed: %s\n",
Expand Down
6 changes: 4 additions & 2 deletions libnvme/src/nvme/private-fabrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,8 @@ size_t libnvmf_get_entity_version(char *buffer, size_t bufsz);
* connected controller. Called from the connect path once the kernel returns
* instance=N.
*/
int libnvmf_registry_create_instance(int instance, const char *owner);
int libnvmf_registry_create_instance(struct libnvme_global_ctx *ctx,
int instance, const char *owner);

/**
* libnvmf_registry_delete_instance - Remove the registry entry for a
Expand All @@ -229,4 +230,5 @@ int libnvmf_registry_create_instance(int instance, const char *owner);
* held the same instance number before it was recycled. ENOENT is silently
* ignored.
*/
int libnvmf_registry_delete_instance(int instance);
int libnvmf_registry_delete_instance(struct libnvme_global_ctx *ctx,
int instance);
Loading
Loading