From 01c77480a0ca6a7c9e5d61592bce50956343f1e1 Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Mon, 27 Jan 2025 14:32:30 -0600 Subject: [PATCH 1/4] Add ability to get ZFS object properties * Add common methods and utility functions for getting ZFS properties. * Add a common asdict() method for ZFS resources. The optional output for this method will increase as more options are plumbed into its kwargs. * Expand the module state with references to commonly used objects. * Add proper cleanup method for module state. --- src/py_zfs.c | 1 + src/py_zfs_dataset.c | 10 - src/py_zfs_enum.c | 55 +++- src/py_zfs_prop.c | 615 +++++++++++++++++++++++++++++------ src/py_zfs_resource.c | 179 ++++++++++ src/py_zfs_state.c | 205 +++++++++++- src/py_zfs_state.h | 57 +++- src/truenas_pylibzfs.c | 32 +- src/truenas_pylibzfs.h | 35 +- src/truenas_pylibzfs_enums.h | 15 + 10 files changed, 1077 insertions(+), 127 deletions(-) diff --git a/src/py_zfs.c b/src/py_zfs.c index 00e9ba3..973f488 100644 --- a/src/py_zfs.c +++ b/src/py_zfs.c @@ -61,6 +61,7 @@ void py_zfs_dealloc(py_zfs_t *self) { Py_END_ALLOW_THREADS } + Py_CLEAR(self->module); self->lzh = NULL; pthread_mutex_destroy(&self->zfs_lock); Py_TYPE(self)->tp_free((PyObject *)self); diff --git a/src/py_zfs_dataset.c b/src/py_zfs_dataset.c index 32c9efd..908ecb8 100644 --- a/src/py_zfs_dataset.c +++ b/src/py_zfs_dataset.c @@ -23,11 +23,6 @@ void py_zfs_dataset_dealloc(py_zfs_dataset_t *self) { Py_TYPE(self)->tp_free((PyObject *)self); } -static -PyObject *py_zfs_dataset_as_dict(PyObject *self, PyObject *args) { - Py_RETURN_NONE; -} - static PyObject *py_repr_zfs_dataset(PyObject *self) { @@ -146,11 +141,6 @@ PyGetSetDef zfs_dataset_getsetters[] = { static PyMethodDef zfs_dataset_methods[] = { - { - .ml_name = "asdict", - .ml_meth = py_zfs_dataset_as_dict, - .ml_flags = METH_VARARGS - }, { .ml_name = "iter_filesystems", .ml_meth = (PyCFunction)py_zfs_dataset_iter_filesystems, diff --git a/src/py_zfs_enum.c b/src/py_zfs_enum.c index 40fc39e..2e89fe8 100644 --- a/src/py_zfs_enum.c +++ b/src/py_zfs_enum.c @@ -1,5 +1,6 @@ #include "truenas_pylibzfs.h" +/* Create a dictionary for enum spec for the ZFSType enum */ static PyObject *zfs_type_table_to_dict(void) { @@ -32,6 +33,7 @@ PyObject *zfs_type_table_to_dict(void) return NULL; } +/* Create a dictionary for enum spec for the ZFSProperty enum */ static PyObject *zfs_prop_table_to_dict(void) { @@ -43,9 +45,18 @@ PyObject *zfs_prop_table_to_dict(void) if (dict_out == NULL) return NULL; - for (i=0; i < ARRAY_SIZE(zpool_prop_table); i++) { + for (i=0; i < ARRAY_SIZE(zfs_prop_table); i++) { PyObject *val = NULL; + /* + * For now exclude properties that aren't + * marked as "visible". We can adjust in + * future if there is some reason to expose + * these properties to consumers + */ + if (!zfs_prop_visible(zfs_prop_table[i].prop)) + continue; + val = PyLong_FromLong(zfs_prop_table[i].prop); if (val == NULL) goto fail; @@ -64,6 +75,7 @@ PyObject *zfs_prop_table_to_dict(void) return NULL; } +/* Create a dictionary for enum spec for the ZPOOLProperty enum */ static PyObject *zpool_prop_table_to_dict(void) { @@ -96,6 +108,7 @@ PyObject *zpool_prop_table_to_dict(void) return NULL; } +/* Create a dictionary for enum spec for the ZFSDOSFlag enum */ static PyObject *zfs_dosflag_table_to_dict(void) { @@ -128,6 +141,40 @@ PyObject *zfs_dosflag_table_to_dict(void) return NULL; } +/* Create a dictionary for enum spec for the PropertySource enum */ +static +PyObject *zfs_prop_src_table_to_dict(void) +{ + PyObject *dict_out = NULL; + int err; + uint i; + + dict_out = PyDict_New(); + if (dict_out == NULL) + return NULL; + + for (i=0; i < ARRAY_SIZE(zprop_source_table); i++) { + PyObject *val = NULL; + + val = PyLong_FromLong(zprop_source_table[i].sourcetype); + if (val == NULL) + goto fail; + + err = PyDict_SetItemString(dict_out, + zprop_source_table[i].name, + val); + Py_DECREF(val); + if (err) + goto fail; + } + + return dict_out; +fail: + Py_XDECREF(dict_out); + return NULL; +} + +/* Create a dictionary for enum spec for the ZFSError enum */ static PyObject *zfs_err_table_to_dict(void) { @@ -160,6 +207,7 @@ PyObject *zfs_err_table_to_dict(void) return NULL; } +/* Create a dictionary for enum spec for the ZPOOLStatus enum */ static PyObject *zpool_status_table_to_dict(void) { @@ -299,6 +347,11 @@ py_add_zfs_enums(PyObject *module) if (err) goto out; + err = add_enum(module, intflag_enum, "PropertySource", + zfs_prop_src_table_to_dict, kwargs); + if (err) + goto out; + out: Py_XDECREF(kwargs); Py_XDECREF(int_enum); diff --git a/src/py_zfs_prop.c b/src/py_zfs_prop.c index 720a1d6..38d2b57 100644 --- a/src/py_zfs_prop.c +++ b/src/py_zfs_prop.c @@ -1,133 +1,542 @@ #include "truenas_pylibzfs.h" -#define ZFS_PROP_STR "<" PYLIBZFS_MODULE_NAME ".ZFSProperty name %s value %s>" +/* + * ZFS property implementation for module + * + * The properties are implemented as Struct Sequence Objects in the C API. + * There are three basic Struct Sequence Object types for the purposes of this + * implementation: + * + * truenas_pylibzfs.struct_zfs_property_data: + * ------------------------------------------ + * This Struct Sequence Object contains the actual property value + * and a reference to the source of the property (if requested). + * + * It is statically defined in this file and has the following fields: + * value: typed python object for the value. + * raw: string representing the value + * source: either Struct Sequence Object for source or None type. + * + * truenas_pylibzfs.struct_zfs_property_source: + * -------------------------------------------- + * This Struct Sequence Object contains information about the source + * of the ZFS property. + * + * It is statically defined in this file and has the following fields: + * type: truenas_pylibzfs.PropertySource IntEnum instance representing + * the underlying zprop_source_t for the property. + * value: str indicating source of the property if it is inherited. + * + * truenas_pylibzfs.struct_zfs_property: + * ------------------------------------- + * This Struct Sequence Object contains requested property information + * for a particular ZFS dataset (filesystem, volume, snapshot). + * + * It is *dyanmically created* when the py_zfs_state is initialized + * on module load. This is an absolute requirement since we do not + * have most of our property information until the first libzfs_t handle + * is opened (which initializes the ZFS properties structs). + */ -PyObject *py_zfs_prop_str(PyObject *self) { - py_zfs_prop_t *prop = (py_zfs_prop_t *)self; - if (prop->cname == NULL) { - return (PyUnicode_FromFormat(ZFS_PROP_STR, "", - "")); +#define LIBZFS_NONE_VALUE "none" + +PyStructSequence_Field struct_zfs_prop [] = { + {"value", "Parsed value of the ZFS property"}, + {"raw", "Raw value of the ZFS property (string)"}, + {"source", "Source dataset of the property"}, + {0}, +}; + +PyStructSequence_Desc struct_zfs_prop_type_desc = { + .name = PYLIBZFS_MODULE_NAME ".struct_zfs_property_data", + .fields = struct_zfs_prop, + .doc = "Python ZFS property structure.", + .n_in_sequence = 3 +}; + +PyStructSequence_Field struct_zfs_prop_src [] = { + {"type", "Source type of the ZFS property"}, + {"value", "Source string of ZFS property"}, + {0}, +}; + +PyStructSequence_Desc struct_zfs_prop_src_type_desc = { + .name = PYLIBZFS_MODULE_NAME ".struct_zfs_property_source", + .fields = struct_zfs_prop_src, + .doc = "Python ZFS property source structure.", + .n_in_sequence = 2 +}; + +/* + * Generate some reasonable-ish documentation for our attributes based on ZFS + * information. This function deliberately uses python memory interface to + * simplify cleanup when the state is freed. Do not change malloc methods + * without changing how the state file is cleaned up. + */ +static +char *py_create_prop_doc(const char *name, zfs_prop_t prop) +{ + const char *v = zfs_prop_values(prop); + boolean_t ro = zfs_prop_readonly(prop); + char buf[1024] = {0}; + char *out = NULL; + + if (ro) { + snprintf(buf, sizeof(buf), + "%s: this property is read-only.", + name); + } else if (v) { + snprintf(buf, sizeof(buf), + "%s: property may be set to one of following " + "values: %s.", name, v); + } else if (zfs_prop_is_string(prop)) { + snprintf(buf, sizeof(buf), "%s: property is string.", name); + } else { + snprintf(buf, sizeof(buf), "%s: property is numeric.", name); } - return (PyUnicode_FromFormat(ZFS_PROP_STR, prop->cname, prop->cvalue)); -} -PyObject *py_zfs_prop_get_propid(py_zfs_prop_t *self, void *extra) { - Py_RETURN_NONE; -} + out = PyMem_New(char, strlen(buf) + 1); + if (out == NULL) + return NULL; -PyObject *py_zfs_prop_set_propid(py_zfs_prop_t *self, PyObject *args, - void *extra) { - Py_RETURN_NONE; + memcpy(out, buf, strlen(buf) + 1); + return out; } -PyObject *py_zfs_prop_get_name(py_zfs_prop_t *self, void *extra) { - Py_RETURN_NONE; -} +/* strdup using python memory allocator. This function deliberately uses + * the python memory interface. If this gets replaced by system malloc, + * the developer should also change how the memory is freed on state cleanup. + */ +static +char *pymem_strdup(const char *s) +{ + size_t len = strlen(s) + 1; + void *new = PyMem_Malloc(len); + if (new == NULL) + return NULL; -PyObject *py_zfs_prop_set_name(py_zfs_prop_t *self, PyObject *args, - void *extra) { - Py_RETURN_NONE; + return (char *)memcpy(new, s, len); } -PyObject *py_zfs_prop_get_rawvalue(py_zfs_prop_t *self, void *extra) { - Py_RETURN_NONE; -} +void init_py_struct_prop_state(pylibzfs_state_t *state) +{ + size_t i; + PyTypeObject *obj; + + for (i = 0; i < ARRAY_SIZE(zfs_prop_table); i++) { + /* Leave hidden properties unnamed in our property struct */ + const char *doc = NULL; + const char *name = PyStructSequence_UnnamedField; + zfs_prop_t prop = zfs_prop_table[i].prop; + + if (zfs_prop_visible(prop)) { + name = zfs_prop_to_name(prop); + doc = py_create_prop_doc(name, prop); + } + + /* + * WARNING: name field must be heap-allocated otherwise + * repr method of sequence was seen to fail in runtime + */ + state->struct_prop_fields[i].name = pymem_strdup(name); + state->struct_prop_fields[i].doc = doc; + + /* + * If this malloc failed then we'll be unable to read ZFS + * properties. It's better to just fail spectacularly rather + * than be semi-broken. + */ + PYZFS_ASSERT(state->struct_prop_fields[i].name, "Malloc failure."); + } + + state->struct_zfs_prop_desc = (PyStructSequence_Desc) { + .name = PYLIBZFS_MODULE_NAME ".struct_zfs_property", + .fields = state->struct_prop_fields, + .n_in_sequence = ARRAY_SIZE(zfs_prop_table) + }; + + obj = PyStructSequence_NewType(&state->struct_zfs_prop_desc); + PYZFS_ASSERT(obj, "Failed to allocate struct_zfs_props_type"); + state->struct_zfs_props_type = obj; + + obj = PyStructSequence_NewType(&struct_zfs_prop_type_desc); + PYZFS_ASSERT(obj, "Failed to allocate struct_zfs_prop_type"); -PyObject *py_zfs_prop_get_source(py_zfs_prop_t *self, void *extra) { - Py_RETURN_NONE; + state->struct_zfs_prop_type = obj; + + obj = PyStructSequence_NewType(&struct_zfs_prop_src_type_desc); + PYZFS_ASSERT(obj, "Failed to allocate struct_zfs_prop_src_type"); + + state->struct_zfs_prop_src_type = obj; } -PyObject *py_zfs_prop_set(PyObject *self, PyObject *args) { - Py_RETURN_NONE; +static int +py_zfs_prop_valid_for_type(zfs_prop_t prop, zfs_type_t zfs_type) +{ + if (zfs_prop_valid_for_type(prop, zfs_type, B_FALSE)) + return B_TRUE; + + PyErr_Format( + PyExc_ValueError, + "%s: property is invalid for zfs type: %s", + zfs_prop_to_name(prop), + get_dataset_type(zfs_type) + ); + + return B_FALSE; } -PyObject *py_zfs_prop_get(PyObject *self, PyObject *args) { - Py_RETURN_NONE; +static PyObject* +py_zfs_parse_source(pylibzfs_state_t *state, + py_zfs_obj_t *pyzfs, + zprop_source_t sourcetype, + const char *source) +{ + PyObject *pysrc_type = NULL; + PyObject *pysrc = NULL; + PyObject *out = NULL; + + pysrc_type = py_get_property_source(pyzfs->pylibzfsp, sourcetype); + if (sourcetype == ZPROP_SRC_INHERITED) { + pysrc = PyUnicode_FromString(source); + if (pysrc == NULL) { + Py_DECREF(pysrc_type); + return NULL; + } + } else { + pysrc = Py_NewRef(Py_None); + } + + out = PyStructSequence_New(state->struct_zfs_prop_src_type); + if (out == NULL) { + Py_DECREF(pysrc_type); + Py_DECREF(pysrc); + return NULL; + } + + PyStructSequence_SET_ITEM(out, 0, pysrc_type); + PyStructSequence_SET_ITEM(out, 1, pysrc); + return out; } -PyObject *py_zfs_prop_asdict(PyObject *self, PyObject *args) { - Py_RETURN_NONE; +static +PyObject *py_parse_zfs_prop(zfs_prop_t prop, char *propbuf, PyObject *raw) +{ + PyObject *out = NULL; + char *pend; + + /* literal "none", convert to None type */ + if (strcmp(propbuf, LIBZFS_NONE_VALUE) == 0) { + Py_RETURN_NONE; + } + + /* Begin custom property parsers */ + + /* End custom property parsers */ + + /* Generic parser based on property type */ + if (zfs_prop_is_string(prop)) { + // For strings, just create new reference to original value + return Py_NewRef(raw); + } else { + /* + * If for some reason we have a decimal point in raw string + * convert it to a python float + */ + if (strstr(propbuf, ".")) + return PyFloat_FromString(raw); + + out = PyLong_FromString(propbuf, &pend, 10); + if (out == NULL) { + return NULL; + } + + if (pend != (propbuf + strlen(propbuf))) { + PyErr_Format( + PyExc_ValueError, + "%s: failed to parse value [%s] as " + "a numeric value.", + zfs_prop_to_name(prop), + propbuf + ); + Py_DECREF(out); + return NULL; + } + } + + return out; } -PyObject *py_zfs_prop_refresh(PyObject *self, PyObject *args) { - Py_RETURN_NONE; +static +PyObject* py_zfs_get_prop(pylibzfs_state_t *state, + py_zfs_obj_t *pyzfs, + zfs_prop_t prop, + boolean_t get_source) +{ + PyObject *out = NULL; + PyObject *raw = NULL; + PyObject *parsed = NULL; + PyObject *source = NULL; + zprop_source_t sourcetype; + char propbuf[ZFS_MAXPROPLEN]; + char sourcebuf[ZFS_MAX_DATASET_NAME_LEN] = {0}; + int err; + + Py_BEGIN_ALLOW_THREADS + /* + * In some edge cases this libzfs function call + * will write a generally useless message into libzfs + * error buffer. This means we need to take lock + * in order to prevent corruption, but we don't need + * to actually look at it. + */ + PY_ZFS_LOCK(pyzfs->pylibzfsp); + if (get_source) { + err = zfs_prop_get(pyzfs->zhp, + prop, + propbuf, + sizeof(propbuf), + &sourcetype, + sourcebuf, + sizeof(sourcebuf), + B_TRUE); + } else { + err = zfs_prop_get(pyzfs->zhp, + prop, + propbuf, + sizeof(propbuf), + &sourcetype, + sourcebuf, + sizeof(sourcebuf), + B_TRUE); + } + PY_ZFS_UNLOCK(pyzfs->pylibzfsp); + Py_END_ALLOW_THREADS + + if (err) { + if (!py_zfs_prop_valid_for_type(prop, pyzfs->ctype)) + return NULL; + + PyErr_Format( + PyExc_RuntimeError, + "%s: failed to get property for dataset.", + zfs_prop_to_name(prop) + ); + return NULL; + } + + raw = PyUnicode_FromString(propbuf); + if (raw == NULL) + return NULL; + + parsed = py_parse_zfs_prop(prop, propbuf, raw); + if (parsed == NULL) { + Py_DECREF(raw); + return NULL; + } + + if (get_source) { + source = py_zfs_parse_source(state, pyzfs, sourcetype, sourcebuf); + if (source == NULL) { + Py_DECREF(raw); + Py_DECREF(parsed); + return NULL; + } + } else { + /* + * set to None type since we can't safely leave NULL values in + * sequence objects (for instance, can lead to this can cause + * json.dumps to segfault). + */ + source = Py_NewRef(Py_None); + } + + out = PyStructSequence_New(state->struct_zfs_prop_type); + if (out == NULL) { + Py_DECREF(raw); + Py_DECREF(parsed); + Py_DECREF(source); + return NULL; + + } + + PyStructSequence_SET_ITEM(out, 0, parsed); + PyStructSequence_SET_ITEM(out, 1, raw); + PyStructSequence_SET_ITEM(out, 2, source); + + return out; } -PyObject *py_zfs_prop_new(PyTypeObject *type, PyObject *args, - PyObject *kwds) { - py_zfs_prop_t *self = NULL; - self = (py_zfs_prop_t *)type->tp_alloc(type, 0); - return ((PyObject *)self); +PyObject *py_zfs_get_properties(py_zfs_obj_t *pyzfs, + PyObject *prop_set, + boolean_t get_source) +{ + pylibzfs_state_t *state = py_get_module_state(pyzfs->pylibzfsp); + PyObject *out = NULL; + size_t idx; + int rv; + + out = PyStructSequence_New(state->struct_zfs_props_type); + if (out == NULL) + goto fail; + + /* + * Note that the following iterates the property table and checks + * whether the properties set contains the property. This means + * that set contents that are the wrong type are ignored, and was + * a deliberate choice to simplify logic and speed up the function. + * + * The key thing we want to avoid here is having any of the struct + * members set to NULL (since that will eventually cause a crash). + */ + for (idx = 0; idx < ARRAY_SIZE(zfs_prop_table); idx++) { + PyObject *enum_obj = state->zfs_prop_enum_tbl[idx].obj; + PyObject *pyprop; + + /* + * Requested properties will have type struct_zfs_property_data + * Unrequested properties will have python None type + * + * If the ZFS property has a value of None then you will have + * ..raw = "none" + * ..value = None + */ + + if (enum_obj == NULL) { + // property is not visible and so we don't expose it to + // API consumers; however, we must not leave NULL otherwise + // it can lead to segfault when doing JSON serialization. + PyStructSequence_SET_ITEM(out, idx, Py_NewRef(Py_None)); + continue; + } + + rv = PySet_Contains(prop_set, enum_obj); + if (rv == -1) { + goto fail; + } else if (rv == 0) { + // entry doesn't exist in set. Set to None + PyStructSequence_SET_ITEM(out, idx, Py_NewRef(Py_None)); + continue; + } + + // At this point the set contains the property + pyprop = py_zfs_get_prop(state, + pyzfs, + state->zfs_prop_enum_tbl[idx].type, + get_source); + if (pyprop == NULL) { + goto fail; + } + + PyStructSequence_SET_ITEM(out, idx, pyprop); + } + + return out; +fail: + Py_XDECREF(out); + return NULL; } -int py_zfs_prop_init(PyObject *type, PyObject *args, PyObject *kwds) { - return (0); +/* + * The functions below are for conversion of the above struct sequence objects + * into python dictionaries. + */ +static +PyObject *py_zfs_src_to_dict(PyObject *pysrc) +{ + int idx; + PyObject *out = NULL; + + out = PyDict_New(); + if (out == NULL) + return NULL; + + for (idx = 0; idx < struct_zfs_prop_src_type_desc.n_in_sequence; idx++) { + const char *name = struct_zfs_prop_src[idx].name; + PyObject *value = PyStructSequence_GET_ITEM(pysrc, idx); + + if (PyDict_SetItemString(out, name, value ? value : Py_None)) { + Py_DECREF(out); + return NULL; + } + } + + return out; } -void py_zfs_prop_dealloc(py_zfs_prop_t *self) { - Py_TYPE(self)->tp_free((PyObject *)self); +static +PyObject *py_zfs_prop_to_dict(PyObject *pyprop) +{ + int idx; + PyObject *out = NULL; + int err; + + out = PyDict_New(); + if (out == NULL) + return NULL; + + for (idx = 0; idx < struct_zfs_prop_type_desc.n_in_sequence; idx++) { + const char *name = struct_zfs_prop[idx].name; + PyObject *value = PyStructSequence_GET_ITEM(pyprop, idx); + + if ((value != Py_None) && (strcmp(name, "source") == 0)) { + PyObject *src_dict = py_zfs_src_to_dict(value); + if (src_dict == NULL) { + Py_DECREF(out); + return NULL; + } + err = PyDict_SetItemString(out, name, src_dict); + Py_DECREF(src_dict); + } else { + err = PyDict_SetItemString(out, name, value); + } + if (err) { + Py_DECREF(out); + return NULL; + } + } + return out; } -PyGetSetDef zfs_prop_getsetters[] = { - { - .name = "propid", - .get = (getter)py_zfs_prop_get_propid, - .set = (setter)py_zfs_prop_set_propid - }, - { - .name = "name", - .get = (getter)py_zfs_prop_get_name - }, - { - .name = "value", - .get = (getter)py_zfs_prop_get_name, - .set = (setter)py_zfs_prop_set_name, - }, - { - .name = "raw_value", - .get = (getter)py_zfs_prop_get_rawvalue - }, - { - .name = "source", - .get = (getter)py_zfs_prop_get_source - }, - { .name = NULL } -}; +PyObject *py_zfs_props_to_dict(py_zfs_obj_t *pyzfs, PyObject *pyprops) +{ + int idx; + PyObject *out = NULL; + pylibzfs_state_t *state = py_get_module_state(pyzfs->pylibzfsp); -PyMethodDef zfs_prop_methods[] = { - { - .ml_name = "set", - .ml_meth = py_zfs_prop_set, - .ml_flags = METH_VARARGS - }, - { - .ml_name = "get", - .ml_meth = py_zfs_prop_get, - .ml_flags = METH_VARARGS - }, - { - .ml_name = "asdict", - .ml_meth = py_zfs_prop_asdict, - .ml_flags = METH_VARARGS - }, - { - .ml_name = "refresh", - .ml_meth = py_zfs_prop_refresh, - .ml_flags = METH_VARARGS - }, - { NULL, NULL, 0, NULL } -}; + out = PyDict_New(); + if (out == NULL) + return NULL; -PyTypeObject ZFSProperty = { - .tp_name = "ZFSProperty", - .tp_basicsize = sizeof (py_zfs_prop_t), - .tp_methods = zfs_prop_methods, - .tp_getset = zfs_prop_getsetters, - .tp_new = py_zfs_prop_new, //PyType_GenericNew, - .tp_init = py_zfs_prop_init, - .tp_doc = "ZFSProperty", - .tp_dealloc = (destructor)py_zfs_prop_dealloc, - .tp_str = py_zfs_prop_str, - .tp_repr = py_zfs_prop_str, - .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, -}; + for (idx = 0; idx < state->struct_zfs_prop_desc.n_in_sequence; idx++) { + const char *name = state->struct_prop_fields[idx].name; + PyObject *pyprop = NULL; + PyObject *value = NULL; + int err; + + // Check if this is a hidden property + if (strcmp(name, PyStructSequence_UnnamedField) == 0) + continue; + + value = PyStructSequence_GET_ITEM(pyprops, idx); + if (value == Py_None) + continue; + + pyprop = py_zfs_prop_to_dict(value); + if (pyprop == NULL) { + Py_DECREF(out); + return NULL; + } + + err = PyDict_SetItemString(out, name, pyprop); + Py_DECREF(pyprop); + + if (err) { + Py_DECREF(out); + return NULL; + + } + } + + return out; +} diff --git a/src/py_zfs_resource.c b/src/py_zfs_resource.c index 6620b31..e464d59 100644 --- a/src/py_zfs_resource.c +++ b/src/py_zfs_resource.c @@ -48,6 +48,173 @@ PyObject *py_zfs_resource_userspace(PyObject *self, PyObject *args) { Py_RETURN_NONE; } +PyDoc_STRVAR(py_zfs_resource_get_properties__doc__, +"asdict(*, properties, get_source=False) -> " +"truenas_pylibzfs.struct_zfs_property\n\n" +"-------------------------------------\n\n" +"Get the specified properties of a given ZFS resource.\n\n" +"" +"Parameters\n" +"----------\n" +"properties: set, required\n" +" Set of truenas_pylibzfs.ZFSProperty properties to retrieve.\n\n" +"get_source: bool, optional, default=False\n" +" Non-default option to retrieve the source information for the returned\n" +" propeties.\n\n" +"" +"Returns\n" +"-------\n" +"truenas_pylibzfs.struct_zfs_property\n" +" Struct sequence object containing the requested property information.\n" +" The requested properties will be represented by truenas_pylibzfs.struct_zfs_property_data\n" +" objects under the respective attributes. Properties that were not\n" +" requested will be set to None type.\n\n" +"" +"Raises:\n" +"-------\n" +"TypeError:\n" +" The specified properties is not a python set.\n\n" +"ValueError:\n" +" One of the specified properties is not supported for the ZFS type of the\n" +" underlying ZFS resource. For example, requesting a zvol property for a\n" +" ZFS filesystem.\n\n" +"RuntimeError:\n" +" An unexpected error occurred while retrieving the value of a ZFS property.\n" +" This should not happen and API consumers experiencing this issue should\n" +" file a bug report against this python library.\n\n" +); +static +PyObject *py_zfs_resource_get_properties(PyObject *self, + PyObject *args_unused, + PyObject *kwargs) +{ + py_zfs_resource_t *res = (py_zfs_resource_t *)self; + PyObject *prop_set = NULL; + boolean_t get_source = B_FALSE; + char *kwnames [] = { + "properties", + "get_source", + NULL + }; + + if (!PyArg_ParseTupleAndKeywords(args_unused, kwargs, + "|$Op", + kwnames, + &prop_set, + &get_source)) { + return NULL; + } + if (prop_set == NULL) { + PyErr_SetString(PyExc_ValueError, + "properties keyword is required."); + return NULL; + } + if (!PySet_Check(prop_set)) { + PyErr_SetString(PyExc_TypeError, + "properties must be a python set."); + return NULL; + } + + return py_zfs_get_properties(&res->obj, prop_set, get_source); +} + +PyDoc_STRVAR(py_zfs_resource_asdict__doc__, +"asdict(*, properties, get_source=False) -> dict\n\n" +"-----------------------------------------------\n\n" +"Get the specified properties of a given ZFS resource.\n\n" +"" +"Parameters\n" +"----------\n" +"properties: set, optional\n" +" Set of truenas_pylibzfs.ZFSProperty properties to retrieve.\n\n" +"get_source: bool, optional, default=False\n" +" Non-default option to retrieve the source information for the returned\n" +" propeties.\n\n" +"" +"Returns\n" +"-------\n" +"python dictionary containing the following keys:\n" +" name: the name of this ZFS resource\n\n" +" pool: the name of the pool with which this resource is associated\n\n" +" type: the ZFS type of this resource\n\n" +" type_enum: the truenas_pylibzfs.ZFSType enum for this resource\n\n" +" createtxg: the ZFS transaction group number in which this resource was\n" +" created\n\n" +" guid: the GUID for this ZFS resource\n" +" properties: dictionary containing property information for requested\n" +" properties. This will be None type if no properties were requested.\n" +"\n" +"Raises:\n" +"-------\n" +"TypeError:\n" +" The specified properties is not a python set.\n\n" +"ValueError:\n" +" One of the specified properties is not supported for the ZFS type of the\n" +" underlying ZFS resource. For example, requesting a zvol property for a\n" +" ZFS filesystem.\n\n" +"RuntimeError:\n" +" An unexpected error occurred while retrieving the value of a ZFS property.\n" +" This should not happen and API consumers experiencing this issue should\n" +" file a bug report against this python library.\n\n" +); +static +PyObject *py_zfs_resource_asdict(PyObject *self, + PyObject *args_unused, + PyObject *kwargs) +{ + py_zfs_resource_t *res = (py_zfs_resource_t *)self; + PyObject *prop_set = NULL; + PyObject *props_dict = NULL; + PyObject *out = NULL; + boolean_t get_source = B_FALSE; + char *kwnames [] = { + "properties", + "get_source", + NULL + }; + + if (!PyArg_ParseTupleAndKeywords(args_unused, kwargs, + "|$Op", + kwnames, + &prop_set, + &get_source)) { + return NULL; + } + + if (prop_set != NULL) { + PyObject *zfsprops = NULL; + + if (!PySet_Check(prop_set)) { + PyErr_SetString(PyExc_TypeError, + "properties must be a set."); + return NULL; + } + zfsprops = py_zfs_get_properties(&res->obj, prop_set, get_source); + if (zfsprops == NULL) + return NULL; + + props_dict = py_zfs_props_to_dict(&res->obj, zfsprops); + Py_CLEAR(zfsprops); + if (props_dict == NULL) + return NULL; + } + + out = Py_BuildValue( + "{s:O,s:O,s:O,s:O,s:O,s:O,s:O}", + "name", res->obj.name, + "pool", res->obj.pool_name, + "type", res->obj.type, + "type_enum", res->obj.type_enum, + "createtxg", res->obj.createtxg, + "guid", res->obj.guid, + "properties", props_dict ? props_dict : Py_None + ); + + Py_XDECREF(props_dict); + return out; +} + + static PyMethodDef zfs_resource_methods[] = { { @@ -60,6 +227,18 @@ PyMethodDef zfs_resource_methods[] = { .ml_meth = py_zfs_resource_update_properties, .ml_flags = METH_VARARGS }, + { + .ml_name = "get_properties", + .ml_meth = (PyCFunction)py_zfs_resource_get_properties, + .ml_flags = METH_VARARGS | METH_KEYWORDS, + .ml_doc = py_zfs_resource_get_properties__doc__ + }, + { + .ml_name = "asdict", + .ml_meth = (PyCFunction)py_zfs_resource_asdict, + .ml_flags = METH_VARARGS | METH_KEYWORDS, + .ml_doc = py_zfs_resource_asdict__doc__ + }, { .ml_name = "userspace", .ml_meth = py_zfs_resource_userspace, diff --git a/src/py_zfs_state.c b/src/py_zfs_state.c index be1c482..55ec912 100644 --- a/src/py_zfs_state.c +++ b/src/py_zfs_state.c @@ -10,7 +10,7 @@ int setup_zfs_type(PyObject *module, pylibzfs_state_t *state) if (pyenum == NULL) return -1; - for (i = 0; i < ARRAY_SIZE(state->zfs_type_enum); i++) { + for (i = 0; i < ARRAY_SIZE(state->zfs_type_enum_tbl); i++) { PyObject *enum_val, *enum_key, *name; enum_key = Py_BuildValue("i", zfs_type_table[i].type); @@ -28,18 +28,120 @@ int setup_zfs_type(PyObject *module, pylibzfs_state_t *state) goto fail; } - state->zfs_type_enum[i].obj = enum_val; - state->zfs_type_enum[i].type = zfs_type_table[i].type; - state->zfs_type_enum[i].name = name; + state->zfs_type_enum_tbl[i].obj = enum_val; + state->zfs_type_enum_tbl[i].type = zfs_type_table[i].type; + state->zfs_type_enum_tbl[i].name = name; } + state->zfs_type_enum = pyenum; return 0; fail: - for (j = 0; j < i; j++) - Py_CLEAR(state->zfs_type_enum[j].obj); + for (j = 0; j < i; j++) { + Py_CLEAR(state->zfs_type_enum_tbl[j].obj); + Py_CLEAR(state->zfs_type_enum_tbl[j].name); + } + + Py_DECREF(pyenum); + return -1; +} + +static +int setup_zfs_prop_type(PyObject *module, pylibzfs_state_t *state) +{ + uint i, j; + PyObject *pyenum; + + pyenum = PyObject_GetAttrString(module, "ZFSProperty"); + if (pyenum == NULL) + return -1; + + for (i = 0; i < ARRAY_SIZE(state->zfs_prop_enum_tbl); i++) { + PyObject *enum_val, *enum_key, *name; + zfs_prop_t prop = zfs_prop_table[i].prop; + if (!zfs_prop_visible(prop)) + continue; + + enum_key = Py_BuildValue("i", zfs_prop_table[i].prop); + if (enum_key == NULL) { + goto fail; + } + + enum_val = PyObject_CallOneArg(pyenum, enum_key); + Py_DECREF(enum_key); + if (enum_val == NULL) { + goto fail; + } + + name = PyObject_GetAttrString(enum_val, "name"); + if (name == NULL) { + goto fail; + } + + state->zfs_prop_enum_tbl[i].obj = enum_val; + state->zfs_prop_enum_tbl[i].type = prop; + state->zfs_prop_enum_tbl[i].name = name; + } + + state->zfs_type_enum = pyenum; + return 0; + +fail: + for (j = 0; j < i; j++) { + Py_CLEAR(state->zfs_prop_enum_tbl[j].obj); + Py_CLEAR(state->zfs_prop_enum_tbl[j].name); + } + + Py_DECREF(pyenum); + return -1; +} + +static +int setup_property_source_type(PyObject *module, pylibzfs_state_t *state) +{ + uint i, j; + PyObject *pyenum; + + pyenum = PyObject_GetAttrString(module, "PropertySource"); + if (pyenum == NULL) + return -1; + + for (i = 0; i < ARRAY_SIZE(state->zfs_prop_src_enum_tbl); i++) { + PyObject *enum_val, *enum_key, *name; + zprop_source_t st = zprop_source_table[i].sourcetype; + + enum_key = Py_BuildValue("i", zprop_source_table[i].sourcetype); + if (enum_key == NULL) { + goto fail; + } + enum_val = PyObject_CallOneArg(pyenum, enum_key); + Py_DECREF(enum_key); + if (enum_val == NULL) { + goto fail; + } + + name = PyObject_GetAttrString(enum_val, "name"); + if (name == NULL) { + goto fail; + } + + state->zfs_prop_src_enum_tbl[i].obj = enum_val; + state->zfs_prop_src_enum_tbl[i].type = st; + state->zfs_prop_src_enum_tbl[i].name = name; + } + + state->zfs_property_src_enum = pyenum; + return 0; + +fail: + for (j = 0; j < i; j++) { + Py_CLEAR(state->zfs_prop_src_enum_tbl[j].obj); + Py_CLEAR(state->zfs_prop_src_enum_tbl[j].name); + } + Py_DECREF(pyenum); return -1; + } pylibzfs_state_t *py_get_module_state(py_zfs_t *zfs) @@ -52,6 +154,18 @@ pylibzfs_state_t *py_get_module_state(py_zfs_t *zfs) return state; } +static +void py_get_enum_instances(PyObject *module, pylibzfs_state_t *state) +{ + PyObject *pyenum; + + pyenum = PyObject_GetAttrString(module, "ZFSProperty"); + PYZFS_ASSERT(pyenum, "Failed to get instance of ZFSProperty enum"); + + state->zfs_property_enum = pyenum; + +} + int init_py_zfs_state(PyObject *module) { int err; @@ -66,6 +180,15 @@ int init_py_zfs_state(PyObject *module) err = setup_zfs_type(module, state); PYZFS_ASSERT(err == 0, "Failed to setup ZFS type in module state."); + err = setup_property_source_type(module, state); + PYZFS_ASSERT(err == 0, "Failed to setup Property Source type in module state."); + + err = setup_zfs_prop_type(module, state); + PYZFS_ASSERT(err == 0, "Failed to setup Property type in module state."); + + py_get_enum_instances(module, state); + + init_py_struct_prop_state(state); return 0; } @@ -77,14 +200,15 @@ PyObject *py_get_zfs_type(py_zfs_t *zfs, zfs_type_t type, PyObject **name_out) state = py_get_module_state(zfs); - for (i = 0; i < ARRAY_SIZE(state->zfs_type_enum); i++) { + for (i = 0; i < ARRAY_SIZE(state->zfs_type_enum_tbl); i++) { if (zfs_type_table[i].type == type) { - out = state->zfs_type_enum[i].obj; + out = state->zfs_type_enum_tbl[i].obj; if (name_out != NULL) { - name = state->zfs_type_enum[i].name; + name = state->zfs_type_enum_tbl[i].name; PYZFS_ASSERT(name, "Failed to get name ref."); } + break; } } @@ -94,3 +218,66 @@ PyObject *py_get_zfs_type(py_zfs_t *zfs, zfs_type_t type, PyObject **name_out) return Py_NewRef(out); } + +PyObject *py_get_property_source(py_zfs_t *zfs, zprop_source_t sourcetype) +{ + PyObject *out = NULL; + pylibzfs_state_t *state = NULL; + + uint i; + + state = py_get_module_state(zfs); + + for (i = 0; i < ARRAY_SIZE(state->zfs_prop_src_enum_tbl); i++) { + if (zprop_source_table[i].sourcetype == sourcetype) { + out = state->zfs_prop_src_enum_tbl[i].obj; + break; + } + } + + PYZFS_ASSERT(out, "Failed to get reference for zprop_source_t enum"); + + return Py_NewRef(out); +} + +/* WARNING: this should only be called from m_clear for module */ +void free_py_zfs_state(PyObject *module) +{ + pylibzfs_state_t *state = NULL; + size_t idx; + + state = (pylibzfs_state_t *)PyModule_GetState(module); + if (state == NULL) + return; + + for (idx = 0; idx < ARRAY_SIZE(state->zfs_type_enum_tbl); idx++) { + Py_CLEAR(state->zfs_type_enum_tbl[idx].name); + Py_CLEAR(state->zfs_type_enum_tbl[idx].obj); + } + + for (idx = 0; idx < ARRAY_SIZE(state->zfs_prop_src_enum_tbl); idx++) { + Py_CLEAR(state->zfs_prop_src_enum_tbl[idx].name); + Py_CLEAR(state->zfs_prop_src_enum_tbl[idx].obj); + } + + for (idx = 0; idx < ARRAY_SIZE(state->zfs_prop_enum_tbl); idx++) { + Py_CLEAR(state->zfs_prop_enum_tbl[idx].name); + Py_CLEAR(state->zfs_prop_enum_tbl[idx].obj); + } + + /* doc and name were allocated using python memory interface */ + for (idx = 0; idx < ARRAY_SIZE(state->struct_prop_fields); idx++) { + PyMem_Free((void *)state->struct_prop_fields[idx].name); + state->struct_prop_fields[idx].name = NULL; + + PyMem_Free((void *)state->struct_prop_fields[idx].doc); + state->struct_prop_fields[idx].doc = NULL; + } + + Py_CLEAR(state->struct_zfs_props_type); + Py_CLEAR(state->struct_zfs_prop_type); + Py_CLEAR(state->struct_zfs_prop_src_type); + Py_CLEAR(state->zfs_property_src_enum); + Py_CLEAR(state->zfs_property_enum); + Py_CLEAR(state->zfs_type_enum); +} diff --git a/src/py_zfs_state.h b/src/py_zfs_state.h index 9b24b37..67e8910 100644 --- a/src/py_zfs_state.h +++ b/src/py_zfs_state.h @@ -2,11 +2,64 @@ #define _PYZFS_STATE_H typedef struct { zfs_type_t type; PyObject *obj; PyObject *name;} pystate_zfstype_t; +typedef struct { zfs_prop_t type; PyObject *obj; PyObject *name;} pystate_zfsprop_t; +typedef struct { zprop_source_t type; PyObject *obj; PyObject *name;} pystate_zfspropsrc_t; typedef struct { - /* Per-module instance lookup tables for enum entries */ - pystate_zfstype_t zfs_type_enum[ARRAY_SIZE(zfs_type_table)]; + /* + * Per-module instance lookup tables for enum entries + * (obj, name) must be freed using Py_CLEAR() when freeing + * the python state. + */ + pystate_zfstype_t zfs_type_enum_tbl[ARRAY_SIZE(zfs_type_table)]; + pystate_zfspropsrc_t zfs_prop_src_enum_tbl[ARRAY_SIZE(zprop_source_table)]; + /* + * This is used by py_zfs_prop.c for fast lookup of ZFSProperty enum + * for determining whether to look up property for output. + */ + pystate_zfsprop_t zfs_prop_enum_tbl[ARRAY_SIZE(zfs_prop_table)]; + + /* + * Fields necessary for implementing struct_zfs_props_type + * This array contains malloced strings (.name, .doc) that must be freed + * using PyMem_Free() when module state is freed. + * These are only used for dynamically creating the struct_zfs_props_type. + */ + PyStructSequence_Field struct_prop_fields[ARRAY_SIZE(zfs_prop_table) + 1]; + PyStructSequence_Desc struct_zfs_prop_desc; + + /* + * Named tuple containing all ZFS properties as attributes. + * this is used for generate get_properties() response. + */ + PyTypeObject *struct_zfs_props_type; + + /* + * Reference to named tuple for individual ZFS properties + * - value (parsed raw value) + * - raw (string) + * - source (struct_zfs_prop_src_type or None type) + */ + PyTypeObject *struct_zfs_prop_type; + + /* + * Reference to named tuple for ZFS property source + * - type (PropertySource enum for the source type) + * - value (str or None type) + */ + PyTypeObject *struct_zfs_prop_src_type; + + /* + * References to enums that are available in the base + * of module. We have them in state so that we can + * efficiently use them for validation purposes. + */ + PyObject *zfs_property_src_enum; + PyObject *zfs_property_enum; + PyObject *zfs_type_enum; } pylibzfs_state_t; extern int init_py_zfs_state(PyObject *module); +extern void init_py_struct_prop_state(pylibzfs_state_t *state); +extern void free_py_zfs_state(PyObject *module); #endif /* _PYZFS_STATE_H */ diff --git a/src/truenas_pylibzfs.c b/src/truenas_pylibzfs.c index 6ab04cd..957c210 100644 --- a/src/truenas_pylibzfs.c +++ b/src/truenas_pylibzfs.c @@ -5,7 +5,6 @@ static PyTypeObject *alltypes[] = { &ZFSDataset, &ZFSObject, &ZFSPool, - &ZFSProperty, &ZFSVdev, NULL }; @@ -95,15 +94,44 @@ static PyMethodDef TruenasPylibzfsMethods[] = { {NULL} }; +static int +pylibzfs_module_clear(PyObject *module) +{ + free_py_zfs_state(module); + return 0; +} + +static void +pylibzfs_module_free(void *module) +{ + if (module) + pylibzfs_module_clear((PyObject *)module); +} + /* Module structure */ static struct PyModuleDef truenas_pylibzfs = { .m_base = PyModuleDef_HEAD_INIT, .m_name = PYLIBZFS_MODULE_NAME, .m_doc = PYLIBZFS_MODULE_NAME " provides python bindings for libzfs for TrueNAS", .m_size = sizeof(pylibzfs_state_t), + .m_clear = pylibzfs_module_clear, + .m_free = pylibzfs_module_free, .m_methods = TruenasPylibzfsMethods, }; +static +void py_init_libzfs(void) +{ + libzfs_handle_t *tmplz = NULL; + + // We need to initialize libzfs handle temporarily so that + // zfs and zpool properties get properly initialized so that + // we can build our Struct Sequences with correct values + + tmplz = libzfs_init(); + PYZFS_ASSERT(tmplz, "Failed to initialize libzfs"); + libzfs_fini(tmplz); +} /* Module initialization */ PyMODINIT_FUNC @@ -122,6 +150,8 @@ PyInit_truenas_pylibzfs(void) add_constants(mpylibzfs); + py_init_libzfs(); + zfs_exc = setup_zfs_exception(); if (zfs_exc == NULL) { Py_DECREF(mpylibzfs); diff --git a/src/truenas_pylibzfs.h b/src/truenas_pylibzfs.h index a5bc05a..237b72c 100644 --- a/src/truenas_pylibzfs.h +++ b/src/truenas_pylibzfs.h @@ -144,7 +144,6 @@ extern PyTypeObject ZFS; extern PyTypeObject ZFSDataset; extern PyTypeObject ZFSObject; extern PyTypeObject ZFSPool; -extern PyTypeObject ZFSProperty; extern PyTypeObject ZFSResource; extern PyTypeObject ZFSVdev; @@ -342,4 +341,38 @@ extern PyObject *py_get_zfs_type(py_zfs_t *zfs, zfs_type_t type, PyObject **name * @param[in] obj - pointer to py_zfs_obj_t object */ extern void free_py_zfs_obj(py_zfs_obj_t *obj); + +/* + * @brief get a ref for truenas_pylibzfs.PropertySource object + * + * Return a new reference to a particular truenas_pylibzfs.PropertySource object + * associated with the sourcetype. This is an IntEnum object. + * + * @param[in] zfs - py_zfs_t handle from which to retrieve source reference + * @param[in] sourcetype - type of source. + * @return returns new reference to the PropertySource + */ +extern PyObject *py_get_property_source(py_zfs_t *zfs, zprop_source_t sourcetype); + +/* Provided by py_zfs_props.c */ +/* + * @brief get the properties specified in prop_set for the ZFS object + * + * This function may be used to retrieve the set of truenas_pylibzfs.ZFSProperty + * properties for a given ZFS filesystem, volume, etc. + * + * @param[in] pyzfs - pointer to a py_zfs_obj_t object (filesystem, zvol, snap) + * @param[in] prop_set - PySet object containing properties to retrieve + * @param[in] get_source - boolean indicating whether to retrieve source of prop. + * @return returns pointer to a Struct Sequence Object with specified properties + * + * @note Properties that were not requested will be set to Py_None. + * + * @note GIL must be held while calling this function. + */ +extern PyObject *py_zfs_get_properties(py_zfs_obj_t *pyzfs, + PyObject *prop_set, + boolean_t get_source); + +extern PyObject *py_zfs_props_to_dict(py_zfs_obj_t *pyzfs, PyObject *pyprops); #endif /* _TRUENAS_PYLIBZFS_H */ diff --git a/src/truenas_pylibzfs_enums.h b/src/truenas_pylibzfs_enums.h index 712225e..ee7d23d 100644 --- a/src/truenas_pylibzfs_enums.h +++ b/src/truenas_pylibzfs_enums.h @@ -190,6 +190,7 @@ static const struct { { ZFS_OFFLINE, "ZFS_SPARSE" }, }; + /* ZFS property enum. Does not expose ones marked as obsolete or * not exposed to the user in the ZFS header file */ static const struct { @@ -340,4 +341,18 @@ static const struct { }; _Static_assert(ZPOOL_NUM_PROPS -1 == ZPOOL_PROP_LAST_SCRUBBED_TXG); +static const struct { + zprop_source_t sourcetype; + const char *name; +} zprop_source_table[] = { + { ZPROP_SRC_NONE, "NONE" }, + { ZPROP_SRC_DEFAULT, "DEFAULT" }, + { ZPROP_SRC_TEMPORARY, "TEMPORARY" }, + { ZPROP_SRC_LOCAL, "LOCAL" }, + { ZPROP_SRC_INHERITED, "INHERITED" }, + { ZPROP_SRC_RECEIVED, "RECEIVED" }, +}; +/* Verify that potential sources haven't changed */ +_Static_assert(ZPROP_SRC_ALL == 0x3f); + #endif /* _TRUENAS_PYLIBZFS_ENUMS_H */ From bef78dd2917f00e4bdbfd8bc601cc29cfd753ecf Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Thu, 30 Jan 2025 07:18:19 -0600 Subject: [PATCH 2/4] Prevent property retrieval on simple handles --- src/py_zfs_resource.c | 31 ++++++++++++++++++++++++++++++- src/truenas_pylibzfs.h | 1 + 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/py_zfs_resource.c b/src/py_zfs_resource.c index e464d59..0a80225 100644 --- a/src/py_zfs_resource.c +++ b/src/py_zfs_resource.c @@ -74,6 +74,8 @@ PyDoc_STRVAR(py_zfs_resource_get_properties__doc__, "-------\n" "TypeError:\n" " The specified properties is not a python set.\n\n" +"TypeError:\n" +" The underlying libzfs handle does not support property retrieval.\n\n" "ValueError:\n" " One of the specified properties is not supported for the ZFS type of the\n" " underlying ZFS resource. For example, requesting a zvol property for a\n" @@ -96,6 +98,18 @@ PyObject *py_zfs_resource_get_properties(PyObject *self, "get_source", NULL }; + if (res->is_simple) { + PyErr_SetString(PyExc_TypeError, + "Properties may not be retrieved on " + "simple ZFS handles. Simple handles are " + "created when iterating with the \"fast\" " + "keyword argument. If property information " + "is required, then the method call that " + "created this handle object should be " + "converted to use the slower variant." + ); + return NULL; + } if (!PyArg_ParseTupleAndKeywords(args_unused, kwargs, "|$Op", @@ -148,6 +162,8 @@ PyDoc_STRVAR(py_zfs_resource_asdict__doc__, "-------\n" "TypeError:\n" " The specified properties is not a python set.\n\n" +"TypeError:\n" +" The underlying libzfs handle does not support property retrieval.\n\n" "ValueError:\n" " One of the specified properties is not supported for the ZFS type of the\n" " underlying ZFS resource. For example, requesting a zvol property for a\n" @@ -184,9 +200,22 @@ PyObject *py_zfs_resource_asdict(PyObject *self, if (prop_set != NULL) { PyObject *zfsprops = NULL; + if (res->is_simple) { + PyErr_SetString(PyExc_TypeError, + "Properties may not be retrieved on " + "simple ZFS handles. Simple handles are " + "created when iterating with the \"fast\" " + "keyword argument. If property information " + "is required, then the method call that " + "created this handle object should be " + "converted to use the slower variant." + ); + return NULL; + } + if (!PySet_Check(prop_set)) { PyErr_SetString(PyExc_TypeError, - "properties must be a set."); + "properties must be a set."); return NULL; } zfsprops = py_zfs_get_properties(&res->obj, prop_set, get_source); diff --git a/src/truenas_pylibzfs.h b/src/truenas_pylibzfs.h index 237b72c..3b2935d 100644 --- a/src/truenas_pylibzfs.h +++ b/src/truenas_pylibzfs.h @@ -62,6 +62,7 @@ typedef struct { * zhp: open zfs handle * ctype: type of ZFS object (C enum) * type: Unicode object for ZFS type + * type_enum: truenas_pylibzfs.ZFSType enum for ctype * name: Unicode object of dataset name * guid: Python Int representing ZFS object GUID * createtxg: Python Int TXG in which object created From 55f5efaaaa6bc1dcd4cee9f5682b61c30e03e0d4 Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Thu, 30 Jan 2025 08:18:48 -0600 Subject: [PATCH 3/4] Refresh ZFS properties on simple handles --- src/py_zfs_resource.c | 58 +++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/src/py_zfs_resource.c b/src/py_zfs_resource.c index 0a80225..1616878 100644 --- a/src/py_zfs_resource.c +++ b/src/py_zfs_resource.c @@ -74,8 +74,6 @@ PyDoc_STRVAR(py_zfs_resource_get_properties__doc__, "-------\n" "TypeError:\n" " The specified properties is not a python set.\n\n" -"TypeError:\n" -" The underlying libzfs handle does not support property retrieval.\n\n" "ValueError:\n" " One of the specified properties is not supported for the ZFS type of the\n" " underlying ZFS resource. For example, requesting a zvol property for a\n" @@ -98,18 +96,6 @@ PyObject *py_zfs_resource_get_properties(PyObject *self, "get_source", NULL }; - if (res->is_simple) { - PyErr_SetString(PyExc_TypeError, - "Properties may not be retrieved on " - "simple ZFS handles. Simple handles are " - "created when iterating with the \"fast\" " - "keyword argument. If property information " - "is required, then the method call that " - "created this handle object should be " - "converted to use the slower variant." - ); - return NULL; - } if (!PyArg_ParseTupleAndKeywords(args_unused, kwargs, "|$Op", @@ -129,6 +115,20 @@ PyObject *py_zfs_resource_get_properties(PyObject *self, return NULL; } + if (res->is_simple) { + /* + * We have simple handle that lacks property information. + * This means we _must_ refresh properties before + * generating python object + */ + Py_BEGIN_ALLOW_THREADS + // This call does not touch ZFS errno and + // so we don't need to take mutex + zfs_refresh_properties(res->obj.zhp); + res->is_simple = B_FALSE; + Py_END_ALLOW_THREADS + } + return py_zfs_get_properties(&res->obj, prop_set, get_source); } @@ -162,8 +162,6 @@ PyDoc_STRVAR(py_zfs_resource_asdict__doc__, "-------\n" "TypeError:\n" " The specified properties is not a python set.\n\n" -"TypeError:\n" -" The underlying libzfs handle does not support property retrieval.\n\n" "ValueError:\n" " One of the specified properties is not supported for the ZFS type of the\n" " underlying ZFS resource. For example, requesting a zvol property for a\n" @@ -200,24 +198,26 @@ PyObject *py_zfs_resource_asdict(PyObject *self, if (prop_set != NULL) { PyObject *zfsprops = NULL; - if (res->is_simple) { - PyErr_SetString(PyExc_TypeError, - "Properties may not be retrieved on " - "simple ZFS handles. Simple handles are " - "created when iterating with the \"fast\" " - "keyword argument. If property information " - "is required, then the method call that " - "created this handle object should be " - "converted to use the slower variant." - ); - return NULL; - } - if (!PySet_Check(prop_set)) { PyErr_SetString(PyExc_TypeError, "properties must be a set."); return NULL; } + + if (res->is_simple) { + /* + * We have simple handle that lacks property information. + * This means we _must_ refresh properties before + * generating python object + */ + Py_BEGIN_ALLOW_THREADS + // This call does not touch ZFS errno and + // so we don't need to take mutex + zfs_refresh_properties(res->obj.zhp); + res->is_simple = B_FALSE; + Py_END_ALLOW_THREADS + } + zfsprops = py_zfs_get_properties(&res->obj, prop_set, get_source); if (zfsprops == NULL) return NULL; From b1633b1d7036918d02bec0a7883926ad793f9310 Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Thu, 30 Jan 2025 09:43:50 -0600 Subject: [PATCH 4/4] Take mutex for property refresh --- src/py_zfs_resource.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/py_zfs_resource.c b/src/py_zfs_resource.c index 1616878..99d9d11 100644 --- a/src/py_zfs_resource.c +++ b/src/py_zfs_resource.c @@ -122,9 +122,9 @@ PyObject *py_zfs_resource_get_properties(PyObject *self, * generating python object */ Py_BEGIN_ALLOW_THREADS - // This call does not touch ZFS errno and - // so we don't need to take mutex + PY_ZFS_LOCK(res->obj.pylibzfsp); zfs_refresh_properties(res->obj.zhp); + PY_ZFS_UNLOCK(res->obj.pylibzfsp); res->is_simple = B_FALSE; Py_END_ALLOW_THREADS } @@ -211,9 +211,9 @@ PyObject *py_zfs_resource_asdict(PyObject *self, * generating python object */ Py_BEGIN_ALLOW_THREADS - // This call does not touch ZFS errno and - // so we don't need to take mutex + PY_ZFS_LOCK(res->obj.pylibzfsp); zfs_refresh_properties(res->obj.zhp); + PY_ZFS_UNLOCK(res->obj.pylibzfsp); res->is_simple = B_FALSE; Py_END_ALLOW_THREADS }