Commit 8befe889 authored by Sebastien Gougeaud's avatar Sebastien Gougeaud Committed by Thomas Leibovici
Browse files

refactor: unify medium and device identifiers

Because both media and devices are identified using a family and a
name, and to avoid code duplication, a resource is now identified by a
pho_id field. This data structure is a renamed version of the previous
media_id structure.

The name subfield stands for the resource primary key in the DB:
- the serial string for devices,
- the ident string for media.

This pho_id field and the model field are now part of a Resource data
structure which contains information that are part of both media and

This patch unifies the identifiers for the phobos source code, but not
the protobuf protocol. This one will be done in the next patch.

Change-Id: I2c7de7fa935986b91943299cb82b97778528ac0c
Signed-off-by: default avatarSebastien Gougeaud <>

Reviewed-by: Linter
Tested-by: default avatarJenkins s8open_nr <>
Reviewed-by: default avatarQuentin Bouget <>
Reviewed-by: default avatarThomas Leibovici <>
parent 2910adda
......@@ -2,7 +2,7 @@
* vim:expandtab:shiftwidth=4:tabstop=4:
* All rights reserved (c) 2014-2019 CEA/DAM.
* All rights reserved (c) 2014-2020 CEA/DAM.
* This file is part of Phobos.
......@@ -139,7 +139,7 @@ out:
return rc;
static int _admin_notify(struct admin_handle *adm, enum dev_family family,
static int _admin_notify(struct admin_handle *adm, enum rsc_family family,
const char *name, enum notify_op op)
pho_resp_t *resp;
......@@ -191,7 +191,7 @@ out:
* TODO: admin_device_add will have the responsability to add the device
* to the DSS, to then remove this part of code from the CLI.
int phobos_admin_device_add(struct admin_handle *adm, enum dev_family family,
int phobos_admin_device_add(struct admin_handle *adm, enum rsc_family family,
const char *name)
int rc;
......@@ -206,7 +206,7 @@ int phobos_admin_device_add(struct admin_handle *adm, enum dev_family family,
return 0;
int phobos_admin_format(struct admin_handle *adm, const struct media_id *id,
int phobos_admin_format(struct admin_handle *adm, const struct pho_id *id,
enum fs_type fs, bool unlock)
pho_resp_t *resp;
......@@ -221,8 +221,8 @@ int phobos_admin_format(struct admin_handle *adm, const struct media_id *id, = rid;
req.format->fs = fs;
req.format->unlock = unlock;
req.format->med_id->type = id->type;
req.format->med_id->id = strdup(id->id);
req.format->med_id->type = id->family;
req.format->med_id->id = strdup(id->name);
rc = _send_and_receive(adm, &req, &resp);
if (rc)
......@@ -230,8 +230,8 @@ int phobos_admin_format(struct admin_handle *adm, const struct media_id *id,
if (pho_response_is_format(resp)) {
if (resp->req_id == rid &&
(int)resp->format->med_id->type == (int)id->type &&
!strcmp(resp->format->med_id->id, id->id)) {
(int)resp->format->med_id->type == (int)id->family &&
!strcmp(resp->format->med_id->id, id->name)) {
pho_debug("Format request succeeded");
goto out;
......@@ -41,8 +41,8 @@ import os.path
from abc import ABCMeta, abstractmethod
from phobos.core.const import dev_family2str
from phobos.core.const import PHO_DEV_DIR, PHO_DEV_TAPE, PHO_LIB_SCSI
from phobos.core.const import rsc_family2str
from phobos.core.const import PHO_LIB_SCSI, PHO_RSC_DIR, PHO_RSC_TAPE
from phobos.core.const import (PHO_DEV_ADM_ST_LOCKED, PHO_DEV_ADM_ST_UNLOCKED,
from phobos.core.ffi import DevInfo, MediaInfo
......@@ -493,7 +493,7 @@ class DriveListOptHandler(ListOptHandler):
attr = [x for x in DevInfo().get_display_dict().keys()]
parser.add_argument('-o', '--output', type=lambda t: t.split(','),
help="attributes to output, comma-separated, "
"choose from {" + " ".join(attr) + "} "
"(default: %(default)s)")
......@@ -514,7 +514,7 @@ class MediaListOptHandler(ListOptHandler):
attr = [x for x in MediaInfo().get_display_dict().keys()]
parser.add_argument('-o', '--output', type=lambda t: t.split(','),
help="attributes to output, comma-separated, "
"choose from {" + " ".join(attr) + "} "
"(default: %(default)s)")
......@@ -571,7 +571,7 @@ class BaseResourceOptHandler(DSSInteractHandler):
def family(self):
"""Return family (as a string) for the current instance."""
return dev_family2str(self.cenum)
return rsc_family2str(self.cenum)
def filter(self, ident):
"""Return a list of objects that match the provided identifier."""
......@@ -707,13 +707,13 @@ class MediaOptHandler(BaseResourceOptHandler):
def exec_add(self):
"""Add new media."""
labels = NodeSet.fromlist(self.params.get('res'))
names = NodeSet.fromlist(self.params.get('res'))
fstype = self.params.get('fs').upper()
techno = self.params.get('type', '').upper()
tags = self.params.get('tags', [])
keep_locked = not self.params.get('unlock')
for med in labels:
for med in names:
try:, fstype, techno, med,
tags=tags, locked=keep_locked)
......@@ -722,7 +722,7 @@ class MediaOptHandler(BaseResourceOptHandler):
sys.exit(os.EX_DATAERR)"Added %d media successfully", len(labels))"Added %d media successfully", len(names))
def exec_update(self):
"""Update an existing media"""
......@@ -785,9 +785,9 @@ class MediaOptHandler(BaseResourceOptHandler):
if unlock:
self.logger.debug("Post-unlock enabled")
for label in media_list:"Formatting media '%s'", label)
adm.fs_format(label, fs_type, unlock=unlock)
for name in media_list:"Formatting media '%s'", name)
adm.fs_format(name, fs_type, unlock=unlock)
except EnvironmentError as err:
# XXX add an option to exit on first error
......@@ -894,7 +894,7 @@ class DirOptHandler(MediaOptHandler, DeviceOptHandler):
"""Directory-related options and actions."""
label = 'dir'
descr = 'handle directories'
cenum = PHO_DEV_DIR
cenum = PHO_RSC_DIR
verbs = MediaOptHandler.verbs
def exec_add(self):
......@@ -929,7 +929,7 @@ class DriveOptHandler(DeviceOptHandler):
"""Tape Drive options and actions."""
label = 'drive'
descr = 'handle tape drives (use ID or device path to identify resource)'
cenum = PHO_DEV_TAPE
cenum = PHO_RSC_TAPE
verbs = [
......@@ -971,7 +971,7 @@ class TapeOptHandler(MediaOptHandler):
"""Magnetic tape options and actions."""
label = 'tape'
descr = 'handle magnetic tape (use tape label to identify resource)'
cenum = PHO_DEV_TAPE
cenum = PHO_RSC_TAPE
verbs = [
......@@ -28,10 +28,10 @@ import os
from ctypes import *
from phobos.core.dss import DSSHandle
from phobos.core.ffi import LIBPHOBOS_ADMIN, MediaId, LRS, CommInfo
from phobos.core.const import (PHO_FS_LTFS, PHO_FS_POSIX,
from phobos.core.dss import DSSHandle
from phobos.core.ffi import CommInfo, LIBPHOBOS_ADMIN, LRS, Id
class AdminHandle(Structure):
"""Admin handler"""
......@@ -67,16 +67,16 @@ class Client(object):
"""Format a medium through the LRS layer."""
fs_type = fs_type.lower()
if fs_type == 'ltfs':
dev_type = PHO_DEV_TAPE
rsc_family = PHO_RSC_TAPE
fs_type_enum = PHO_FS_LTFS
elif fs_type == 'posix':
dev_type = PHO_DEV_DIR
rsc_family = PHO_RSC_DIR
fs_type_enum = PHO_FS_POSIX
raise EnvironmentError(errno.EOPNOTSUPP,
"Unknown filesystem type '%s'" % fs_type)
mstruct = MediaId(dev_type, medium_id)
mstruct = Id(rsc_family, medium_id)
rc = LIBPHOBOS_ADMIN.phobos_admin_format(byref(self.handle),
byref(mstruct), fs_type_enum,
......@@ -63,15 +63,15 @@ static PyObject *py_extent_state2str(PyObject *self, PyObject *args)
return Py_BuildValue("s", str_repr);
static PyObject *py_dev_family2str(PyObject *self, PyObject *args)
static PyObject *py_rsc_family2str(PyObject *self, PyObject *args)
enum dev_family family;
enum rsc_family family;
const char *str_repr;
if (!PyArg_ParseTuple(args, "i", &family))
return NULL;
str_repr = dev_family2str(family);
str_repr = rsc_family2str(family);
return Py_BuildValue("s", str_repr);
......@@ -134,7 +134,7 @@ static PyObject *py_str2fs_type(PyObject *self, PyObject *args)
static PyMethodDef ConstMethods[] = {
{"extent_state2str", py_extent_state2str, METH_VARARGS,
"printable extent state name."},
{"dev_family2str", py_dev_family2str, METH_VARARGS,
{"rsc_family2str", py_rsc_family2str, METH_VARARGS,
"printable dev family name."},
{"adm_status2str", py_adm_status2str, METH_VARARGS,
"printable fs status."},
......@@ -169,12 +169,12 @@ PyMODINIT_FUNC initconst(void)
PyModule_AddIntMacro(mod, PHO_EXT_ST_ORPHAN);
PyModule_AddIntMacro(mod, PHO_EXT_ST_LAST);
/* enum dev_family */
PyModule_AddIntMacro(mod, PHO_DEV_INVAL);
PyModule_AddIntMacro(mod, PHO_DEV_DISK);
PyModule_AddIntMacro(mod, PHO_DEV_TAPE);
PyModule_AddIntMacro(mod, PHO_DEV_DIR);
PyModule_AddIntMacro(mod, PHO_DEV_LAST);
/* enum rsc_family */
PyModule_AddIntMacro(mod, PHO_RSC_INVAL);
PyModule_AddIntMacro(mod, PHO_RSC_DISK);
PyModule_AddIntMacro(mod, PHO_RSC_TAPE);
PyModule_AddIntMacro(mod, PHO_RSC_DIR);
PyModule_AddIntMacro(mod, PHO_RSC_LAST);
/* enum lib_type */
PyModule_AddIntMacro(mod, PHO_LIB_INVAL);
......@@ -39,7 +39,7 @@ from phobos.core.const import PHO_MDA_ADM_ST_LOCKED, PHO_MDA_ADM_ST_UNLOCKED
from phobos.core.const import PHO_DEV_ADM_ST_LOCKED, PHO_DEV_ADM_ST_UNLOCKED
from phobos.core.const import DSS_SET_INSERT, DSS_SET_UPDATE, DSS_SET_DELETE
from phobos.core.ffi import DevInfo, MediaInfo, MediaId, MediaStats
from phobos.core.ffi import DevInfo, Id, MediaInfo, MediaStats, Resource
from phobos.core.ffi import LIBPHOBOS
from phobos.core.ldm import ldm_device_query
......@@ -261,17 +261,17 @@ class DeviceManager(BaseObjectManager):
dev_info = DevInfo(state.lds_family, state.lds_model, device_path,
host, state.lds_serial, status)
dev_info = DevInfo(Resource(Id(state.lds_family, state.lds_serial),
state.lds_model), device_path, host, status)
self.insert([dev_info])"Device '%s:%s' successfully added: " \
"model=%s serial=%s (%s)",, device_path, dev_info.model,
dev_info.serial, locked and "locked" or "unlocked"), locked and "locked" or "unlocked")
return, dev_info.serial
def _dss_get(self, hdl, qry_filter, res, res_cnt):
"""Invoke device-specific DSS get method."""
......@@ -294,12 +294,11 @@ class MediaManager(BaseObjectManager):
self.lock_owner_count += 1
def add(self, mtype, fstype, model, label, tags=None, locked=False):
def add(self, family, fstype, model, name, tags=None, locked=False):
"""Insert media into DSS."""
media = MediaInfo() = MediaId(mtype, label)
media.rsc = Resource(Id(family, name), model)
media.fs.type = str2fs_type(fstype)
media.model = model
media.addr_type = PHO_ADDR_HASH1
media.tags = tags or []
if locked:
......@@ -313,7 +312,7 @@ class MediaManager(BaseObjectManager):
self.logger.debug("Media '%s' successfully added: "\
"model=%s fs=%s (%s)",
label, model, fstype,
name, model, fstype,
locked and "locked" or "unlocked")
def _dss_get(self, hdl, qry_filter, res, res_cnt):
......@@ -32,7 +32,7 @@ from ctypes import *
from phobos.core.const import PHO_LABEL_MAX_LEN, PHO_URI_MAX
from phobos.core.const import fs_type2str, fs_status2str
from phobos.core.const import adm_status2str, dev_family2str
from phobos.core.const import adm_status2str, rsc_family2str
......@@ -96,14 +96,26 @@ class LRS(Structure):
('comm', CommInfo)
class Id(Structure):
"""Resource Identifier."""
_fields_ = [
('family', c_int),
('name', c_char * PHO_URI_MAX)
class Resource(Structure):
_fields_ = [
('id', Id),
('model', c_char_p)
class DevInfo(Structure, CLIManagedResourceMixin):
"""DSS device descriptor."""
_fields_ = [
('family', c_int),
('model', c_char_p),
('rsc', Resource),
('path', c_char_p),
('host', c_char_p),
('serial', c_char_p),
('adm_status', c_int),
('lock', DSSLock)
......@@ -112,19 +124,29 @@ class DevInfo(Structure, CLIManagedResourceMixin):
"""Return a dict of available fields and optional display formatters."""
return {
'adm_status': adm_status2str,
'family': dev_family2str,
'family': rsc_family2str,
'host': None,
'model': None,
'path': None,
'label': None,
'name': None,
'lock_status': None,
'lock_ts': None
def label(self):
"""Wrapper to get label"""
return self.serial
def name(self):
"""Wrapper to get name"""
def family(self):
"""Wrapper to get family"""
def model(self):
"""Wrapper to get model"""
return self.rsc.model
def lock_status(self):
......@@ -160,13 +182,6 @@ class Tags(Structure):
class MediaId(Structure):
"""Generic media identifier."""
_fields_ = [
('type', c_int),
('id', c_char * PHO_URI_MAX)
class MediaFS(Structure):
"""Media filesystem descriptor."""
_fields_ = [
......@@ -190,9 +205,8 @@ class MediaStats(Structure):
class MediaInfo(Structure, CLIManagedResourceMixin):
"""DSS media descriptor."""
_fields_ = [
('id', MediaId),
('rsc', Resource),
('addr_type', c_int),
('model', c_char_p),
('adm_status', c_int),
('fs', MediaFS),
('stats', MediaStats),
......@@ -206,7 +220,7 @@ class MediaInfo(Structure, CLIManagedResourceMixin):
'adm_status': adm_status2str,
'addr_type': None,
'model': None,
'label': None,
'name': None,
'tags': None,
'lock_status': None,
'lock_ts': None
......@@ -230,12 +244,18 @@ class MediaInfo(Structure, CLIManagedResourceMixin):
def lock_ts(self):
"""Wrappert to get lock timestamp"""
"""Wrapper to get lock timestamp"""
return self.lock.lock_ts
def label(self):
def name(self):
"""Wrapper to get medium name"""
def model(self):
"""Wrapper to get medium model"""
return self.rsc.model
def expanded_fs_info(self):
......@@ -62,7 +62,7 @@ def xml_dump(data, item_type='item'):
def human_dump(data):
"""Convert a list of dictionaries to an identifier list text"""
out = "\n".join(str(item['label']) for item in data)
out = "\n".join(str(item['name']) for item in data)
return out
def human_pretty_dump(data):
......@@ -109,7 +109,7 @@ def dump_object_list(objs, item_type, attr=None, fmt="human"):
'human': human_dump,
if attr != ['label']:
if attr != ['name']:
formats['human'] = human_pretty_dump
objlist = filter_display_dict(objs, attr)
......@@ -35,7 +35,7 @@ from socket import gethostname
from phobos.cli import PhobosActionContext
from phobos.core.dss import Client, MediaManager
from phobos.core.ffi import DevInfo
from phobos.core.const import PHO_DEV_DIR, PHO_DEV_TAPE
from phobos.core.const import PHO_RSC_DIR, PHO_RSC_TAPE
def gethostname_short():
"""Return short hostname"""
......@@ -92,12 +92,12 @@ class CLIParametersTest(unittest.TestCase):
self.check_cmdline_valid(['dir', 'add', 'A', 'B', 'C'])
self.check_cmdline_valid(['dir', 'list', 'A,B,C', '-o', 'all'])
self.check_cmdline_valid(['dir', 'list', 'A,B,C', '-o', '*'])
self.check_cmdline_valid(['dir', 'list', 'A,B,C', '-o', 'label,family'])
self.check_cmdline_valid(['dir', 'list', 'A,B,C', '-o', 'name,family'])
self.check_cmdline_valid(['tape', 'add', '-t', 'LTO5', 'I,J,K'])
self.check_cmdline_valid(['tape', 'list', 'I,J,K', '-o', 'all'])
self.check_cmdline_valid(['tape', 'list', 'I,J,K', '-o', '*'])
self.check_cmdline_valid(['tape', 'list', 'I,J,K', '-o',
# Test invalid object and invalid verb
self.check_cmdline_exit(['voynichauthor', 'list'], code=2)
......@@ -28,8 +28,8 @@ import os
from random import randint
from phobos.core.dss import Client
from phobos.core.ffi import MediaInfo, DevInfo
from phobos.core.const import dev_family2str, PHO_DEV_DIR, PHO_DEV_TAPE
from phobos.core.ffi import DevInfo, Id, MediaInfo, Resource
from phobos.core.const import PHO_RSC_DIR, PHO_RSC_TAPE, rsc_family2str
class DSSClientTest(unittest.TestCase):
......@@ -57,7 +57,7 @@ class DSSClientTest(unittest.TestCase):
with Client() as client:
for fam in ('tape', 'disk', 'dir'):
for dev in client.devices.get(family=fam):
self.assertEqual(dev_family2str(, fam)
self.assertEqual(rsc_family2str(, fam)
def test_list_media(self):
"""List media."""
......@@ -68,20 +68,20 @@ class DSSClientTest(unittest.TestCase):
# Check negative indexation
medias =
self.assertEqual(medias[0].label, medias[-len(medias)].label)
self.assertEqual(medias[len(medias) - 1].label, medias[-1].label)
self.assertEqual(medias[0].name, medias[-len(medias)].name)
self.assertEqual(medias[len(medias) - 1].name, medias[-1].name)
def test_list_media_by_tags(self):
with Client() as client:, 'POSIX', None, 'm0'), 'POSIX', None, 'm1', tags=['foo']), 'POSIX', None, 'm2', \, 'POSIX', None, 'm0'), 'POSIX', None, 'm1', tags=['foo']), 'POSIX', None, 'm2', \
tags=['foo', 'bar']), 'POSIX', None, 'm3', \, 'POSIX', None, 'm3', \
tags=['goo', 'foo']), 'POSIX', None, 'm4', \, 'POSIX', None, 'm4', \
tags=['bar', 'goo']), 'POSIX', None, 'm5', \, 'POSIX', None, 'm5', \
tags=['foo', 'bar', 'goo'])
n_tags = {'foo': 4, 'bar': 3, 'goo': 3}
n_bar_foo = 2
......@@ -105,12 +105,10 @@ class DSSClientTest(unittest.TestCase):
with Client() as client:
insert_list = []
for i in range(10):
dev = DevInfo() = PHO_DEV_DIR
dev.model = ''
dev.path = '/tmp/test_%d' % randint(0, 1000000) = 'localhost'
dev.serial = '__TEST_MAGIC_%d' % randint(0, 1000000)
id = Id(PHO_RSC_DIR, '__TEST_MAGIC_%d' % randint(0, 1000000))
rsc = Resource(id=id, model='')
dev = DevInfo(rsc=rsc, path='/tmp/test_%d' % randint(0,1000000),
......@@ -118,11 +116,12 @@ class DSSClientTest(unittest.TestCase):
# now retrieve them one by one and check serials
for dev in insert_list:
res = client.devices.get(serial=dev.serial)
res = client.devices.get(
for retrieved_dev in res:
# replace with assertIsInstance when we drop pre-2.7 support
self.assertTrue(isinstance(retrieved_dev, dev.__class__))
self.assertEqual(retrieved_dev.serial, dev.serial)
......@@ -131,7 +130,7 @@ class DSSClientTest(unittest.TestCase):
# Not the best place to test SQL escaping, but most convenient one.
with Client() as client:
PHO_DEV_TAPE, "LTFS", "lto8", "TAPE_SQLI_0'; <sqli>",
PHO_RSC_TAPE, "LTFS", "lto8", "TAPE_SQLI_0'; <sqli>",
def test_manipulate_empty(self):
......@@ -151,11 +150,11 @@ class DSSClientTest(unittest.TestCase):
"""Test media lock and unlock wrappers"""
with Client() as client:
# Create a dummy media in db
label = '/some/path_%d' % randint(0, 1000000), 'POSIX', None, label, locked=False)
name = '/some/path_%d' % randint(0, 1000000), 'POSIX', None, name, locked=False)
# Get the created media from db
media =[0]
media =[0]
# It should not be locked yet
......@@ -168,7 +167,7 @@ class DSSClientTest(unittest.TestCase):[media])
# Retrieve an up-to-date version
media =[0]
media =[0]
# This one should be locked
......@@ -180,7 +179,7 @@ class DSSClientTest(unittest.TestCase):[media])
# The up-to-date version isn't locked anymore
media =[0]
media =[0]
......@@ -33,12 +33,12 @@
#include <assert.h>
#include <stdbool.h>
bool media_id_equal(const struct media_id *id1, const struct media_id *id2)
bool pho_id_equal(const struct pho_id *id1, const struct pho_id *id2)
if (id1->type != id2->type)
if (id1->family != id2->family)
return false;
if (strcmp(id1->id, id2->id))
if (strcmp(id1->name, id2->name))
return false;
return true;
......@@ -60,11 +60,10 @@ struct dev_info *dev_info_dup(const struct dev_info *dev)
if (!dev_out)
return NULL;
dev_out->family = dev->family;
dev_out->model = strdup_safe(dev->model);