Re: PATCH: pyblock: Make dm.c / dmraid.c handle tables with more then one row correctly

[Joel Granados <jgranado redhat com>]

Have not tested but it looks ok.
On Tue, Feb 24, 2009 at 10:41:50PM +0100, Hans de Goede wrote:
> A devicemapper map's table can have multiple rows (targets in devicemapper
> speak), but we assume their is always only one. This patch updates the C
> part of pyblock to take account that their may be multiple rows in a table
> and makes functions which return / expect a PydmTableObject return / expect
> a list of PydmTableObject's. Maybe we should rename PydmTableObject to
> PydmTableRowObject to reflect it does not abstract a complete table, but only
> one row / target of a complete table.
>
> Together with the mathching python changes this fixes fakeraid jbod / 
> spanning setups.
> ---
>  dm.c     |   54 ++++++++++++-------
>  dmraid.c |  175 +++++++++++++++++++++++++++++++++-----------------------------
>  2 files changed, 129 insertions(+), 100 deletions(-)
>
> diff --git a/dm.c b/dm.c
> index 195afd7..f279cb0 100644
> --- a/dm.c
> +++ b/dm.c
> @@ -872,9 +872,10 @@ pydm_map_simple(PydmMapObject *map, int taskno)
>  }
>
>  static int
> -pydm_map_create(PydmMapObject *map, PydmTableObject *table)
> +pydm_map_create(PydmMapObject *map, PyObject *table)
>  {
>  	struct dm_task *task;
> +	int i;
>
>  	if (!map->name) {
>  		PyErr_SetString(PyExc_ValueError,
> @@ -896,9 +897,14 @@ pydm_map_create(PydmMapObject *map, PydmTableObject *table)
>  		dm_task_set_uuid(task, map->uuid);
>  	python_error_destroy_task(task, -1);
>
> -	dm_task_add_target(task, table->start, table->size,
> -			table->type, table->params);
> -	python_error_destroy_task(task, -1);
> +	for (i = 0; i < PyList_Size(table); i++) {
> +		PydmTableObject *row =
> +			(PydmTableObject *)PyList_GET_ITEM(table, i);
> +
> +		dm_task_add_target(task, row->start, row->size,
> +				   row->type, row->params);
> +		python_error_destroy_task(task, -1);
> +	}
>
>  	if (map->dev) {
>  		PydmDeviceObject *dev = (PydmDeviceObject *)map->dev;
> @@ -930,13 +936,13 @@ pydm_map_init_method(PyObject *self, PyObject 
> *args, PyObject *kwds)
>  	char *kwlist[] = {"name", "table", "uuid", "dev", NULL};
>  	PydmMapObject *map = (PydmMapObject *)self;
>  	PydmDeviceObject *dev = NULL;
> -	PydmTableObject *table = NULL;
> +	PyObject *table = NULL;
>  	char *uuid = NULL, *name = NULL;
>
>  	pydm_map_clear(map);
>
>  	if (!PyArg_ParseTupleAndKeywords(args, kwds, "|zO!zO!:map.__init__",
> -			kwlist, &name, &PydmTable_Type, &table, &uuid,
> +			kwlist, &name, &PyList_Type, &table, &uuid,
>  			&PydmDevice_Type, &dev))
>  		return -1;
>
> @@ -1088,7 +1094,7 @@ pydm_map_get_table(PydmMapObject *map)
>  	struct pydm_map_key key;
>  	int rc;
>  	void *next = NULL;
> -	PydmTableObject *table = NULL;
> +	PyObject *table = NULL, *table_list = NULL;
>
>  	rc = pydm_map_get_best_key(map, &key);
>  	if (rc < 0) {
> @@ -1114,25 +1120,35 @@ pydm_map_get_table(PydmMapObject *map)
>
>  		next = dm_get_next_target(task, next, &start, &length,
>  				&target_type, &params);
> +		if (!target_type) {
> +			PyErr_SetString(PyExc_RuntimeError, "no dm table found");
> +			Py_CLEAR(table_list);
> +			break;
> +		}
>
> +		if (!table_list) {
> +			table_list = PyList_New(0);
> +			if (!table_list)
> +				break;
> +		}
> +
> +		table = PydmTable_FromInfo(start, length, target_type, params);
>  		if (!table) {
> -			python_error_destroy_task(task, NULL);
> -
> -			table = (PydmTableObject *)PydmTable_FromInfo(start,
> -					length, target_type, params);
> -		} else if (PyErr_Occurred()) {
> -			/* this means it happened in one of
> -			   the dm_get_next_target iterations that we didn't
> -			   expect and don't really care about. */
> -			PyErr_Clear();
> +			Py_CLEAR(table_list);
> +			break;
> +		}
> +
> +		rc = PyList_Append(table_list, table);
> +		Py_DECREF(table);
> +		if (rc < 0) {
> +			Py_CLEAR(table_list);
> +			break;
>  		}
>  	} while (next);
> -	if (!table)
> -		PyErr_SetString(PyExc_RuntimeError, "no dm table found");
>
>  	dm_task_update_nodes();
>  	dm_task_destroy(task);
> -	return (PyObject *)table;
> + 	return table_list;
>  }
>
>  static PyObject *
> diff --git a/dmraid.c b/dmraid.c
> index f2f8d9e..5b5cf6c 100644
> --- a/dmraid.c
> +++ b/dmraid.c
> @@ -704,9 +704,9 @@ pydmraid_raidset_get_dm_table(PyObject *self, void *data)
>  	size_t spn;
>
>  	PyObject *m = NULL, *md = NULL;
> -	PyObject *type = NULL, *args = NULL;
> +	PyObject *type = NULL;
>  	PyObject *table = NULL;
> -
> +	PyObject *table_list = NULL;
>  	PyObject *iret = NULL;
>
>  	/* ugh, there's no generic way of telling if this should even
> @@ -715,119 +715,132 @@ pydmraid_raidset_get_dm_table(PyObject *self, void *data)
>  	if (a) {
>  		s = strdupa(a);
>  		free(a);
> -		a = b = s;
>  	}
>  	if (!s) {
>  		PyErr_SetString(PyExc_RuntimeError, "no mapping possible");
>  		return NULL;
>  	}
> -	/* find the beginning of the start */
> -	spn = strspn(s, " \t");
> -	a += spn;
> -	if (*a == '\0')
> -		goto out;
>
> -	b = NULL;
> -	errno = 0;
> -	start = strtoull(a, &b, 10);
> -	if (start == ULLONG_MAX && errno != 0)
> -		goto out;
> -
> -	/* find the length */
> -	spn = strspn(b, " \t");
> -	a = b + spn;
> -	if (!spn || *a == '\0')
> +	/* get the DSO for block.dm loaded */
> +	m = PyImport_ImportModule("block.dm");
> +	if (!m)
>  		goto out;
>
> -	b = NULL;
> -	errno = 0;
> -	length = strtoull(a, &b, 10);
> -	if (length == ULLONG_MAX && errno != 0)
> +	md = PyModule_GetDict(m);
> +	if (!md)
>  		goto out;
>
> -	/* now the dm type */
> -	spn = strspn(b, " \t");
> -	a = b + spn;
> -	if (!spn || *a == '\0')
> +	type = PyDict_GetItemString(md, "table");
> +	if (!type)
>  		goto out;
>
> -	spn = strcspn(a, " \t");
> -	if (!spn)
> +	table_list = PyList_New(0);
> +	if (!table_list)
>  		goto out;
> -	dmtype = strndupa(a, spn);
>
> -	a += spn;
> -	spn = strspn(a, " \t");
> -	a += spn;
> -	if (a)
> -		rule=strdupa(a);
> +	s = strtok(s, "\n");
> +	do {
> +		a = b = s;
> +		/* find the beginning of the start */
> +		spn = strspn(s, " \t");
> +		a += spn;
> +		if (*a == '\0')
> +			goto out;
> +
> +		b = NULL;
> +		errno = 0;
> +		start = strtoull(a, &b, 10);
> +		if (start == ULLONG_MAX && errno != 0)
> +			goto out;
> +
> +		/* find the length */
> +		spn = strspn(b, " \t");
> +		a = b + spn;
> +		if (!spn || *a == '\0')
> +			goto out;
> +
> +		b = NULL;
> +		errno = 0;
> +		length = strtoull(a, &b, 10);
> +		if (length == ULLONG_MAX && errno != 0)
> +			goto out;
> +
> +		/* now the dm type */
> +		spn = strspn(b, " \t");
> +		a = b + spn;
> +		if (!spn || *a == '\0')
> +			goto out;
> +
> +		spn = strcspn(a, " \t");
> +		if (!spn)
> +			goto out;
> +		dmtype = strndupa(a, spn);
> +
> +		a += spn;
> +		spn = strspn(a, " \t");
> +		a += spn;
> +		if (a)
> +			rule=strdupa(a);
>
>  #ifndef ALLOW_NOSYNC
> -	/* XXX FIXME we shouldn't need to do this */
> -	/* transform "core 2 64 nosync" into "core 1 64" */
> -	a = strstr(rule, "core");
> -	if (a) {
> -		/* find the first space */
> -		a += strcspn(a, " ");
> -		/* find the first nonspace */
> -		a += strspn(a, " ");
> -		/* if it's not 2, don't change anything */
> -		if (!strncmp(a, "2 ", 2)) {
> -			/* change "2 " to a "1 " */
> -			a[0]='1';
> -			a+=2;
> -			/* find the end of this arg (the space after "64") */
> +		/* XXX FIXME we shouldn't need to do this */
> +		/* transform "core 2 64 nosync" into "core 1 64" */
> +		a = strstr(rule, "core");
> +		if (a) {
> +			/* find the first space */
>  			a += strcspn(a, " ");
> -			/* .. and the arg we want rid of */
> +			/* find the first nonspace */
>  			a += strspn(a, " ");
> -			/* now find the beginning of our replacement */
> -			b = a + strcspn(a, " ");
> -			b += strspn(b, " ");
> -			/* and now clobber it */
> -			spn = strlen(b);
> -			memmove(a, b, spn+1);
> +			/* if it's not 2, don't change anything */
> +			if (!strncmp(a, "2 ", 2)) {
> +				/* change "2 " to a "1 " */
> +				a[0]='1';
> +				a+=2;
> +				/* find the end of this arg (the space after "64") */
> +				a += strcspn(a, " ");
> +				/* .. and the arg we want rid of */
> +				a += strspn(a, " ");
> +				/* now find the beginning of our replacement */
> +				b = a + strcspn(a, " ");
> +				b += strspn(b, " ");
> +				/* and now clobber it */
> +				spn = strlen(b);
> +				memmove(a, b, spn+1);
> +			}
>  		}
> -	}
>  #endif
> +		table = PyType_GenericNew((PyTypeObject *)type, NULL, NULL);
> +		if (!table)
> +			goto out;
>
> -	args = Py_BuildValue("(LLss)", start, length, dmtype, rule);
> -	if (!args)
> -		goto out;
> -
> -	/* get the DSO for block.dm loaded */
> -	m = PyImport_ImportModule("block.dm");
> -	if (!m)
> -		goto out;
> +		iret = PyObject_CallMethod(table, "__init__", "LLss", start, length, 
> dmtype, rule);
> +		if (!iret)
> +			goto out;
>
> -	md = PyModule_GetDict(m);
> -	if (!md)
> -		goto out;
> +		if (PyList_Append(table_list, table) < 0)
> +			goto out;
>
> -	type = PyDict_GetItemString(md, "table");
> -	if (!type)
> -		goto out;
> +		Py_CLEAR(iret);
> +		Py_CLEAR(table);
> +	} while ((s = strtok(NULL, "\n")));
>
> -	table = PyType_GenericNew((PyTypeObject *)type, args, NULL);
> -	if (!table)
> -		goto out;
> +	Py_DECREF(m);
>
> -	iret = PyObject_CallMethod(table, "__init__", "LLss", start, length, 
> dmtype, rule);
> -	if (!iret) {
> -		Py_DECREF(table);
> -		goto out;
> -	}
> +	return table_list;
>
>  out:
>  	Py_XDECREF(iret);
> -	Py_XDECREF(args);
> -	if (!table && !PyErr_Occurred()) {
> +	Py_XDECREF(table);
> +	Py_XDECREF(table_list);
> +	Py_XDECREF(m);
> +	if (!PyErr_Occurred()) {
>  		if (errno != 0)
>  			PyErr_SetFromErrno(PyExc_SystemError);
>  		else
>  			pyblock_PyErr_Format(PyExc_ValueError,
>  					"invalid map '%s'", s);
>  	}
> -	return table;
> +	return NULL;
>  }
>
>  static PyObject *
> -- 
> 1.6.1.3
>
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list redhat com
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list

-- 
Joel Andres Granados
Brno, Czech Republic, Red Hat.