[PATCH 1/2] lvm2app: Add function to retrieve the origin.

[PATCH 1/2] lvm2app: Add function to retrieve the origin.

[Tony Asleson]

Signed-off-by: Tony Asleson <tasleson@redhat.com>
---
 liblvm/lvm2app.h | 16 ++++++++++++++++
 liblvm/lvm_lv.c  |  5 +++++
 2 files changed, 21 insertions(+)

diff --git a/liblvm/lvm2app.h b/liblvm/lvm2app.h
index 15827ac..369c300 100644
--- a/liblvm/lvm2app.h
+++ b/liblvm/lvm2app.h
@@ -1223,6 +1223,22 @@ const char *lvm_lv_get_name(const lv_t lv);
 const char *lvm_lv_get_attr(const lv_t lv);
 
 /**
+ * Get the origin of a snapshot.
+ *
+ * \memberof lv_t
+ *
+ * The memory allocated for the name is tied to the vg_t handle and will be
+ * released when lvm_vg_close() is called.
+ *
+ * \param   lv
+ * Logical volume handle.
+ *
+ * \return
+ * Null if the logical volume is not a snapshot, else origin name.
+ */
+const char *lvm_lv_get_origin(const lv_t lv);
+
+/**
  * Get the current size in bytes of a logical volume.
  *
  * \memberof lv_t
diff --git a/liblvm/lvm_lv.c b/liblvm/lvm_lv.c
index 9d3ae79..f244a60 100644
--- a/liblvm/lvm_lv.c
+++ b/liblvm/lvm_lv.c
@@ -53,6 +53,11 @@ const char *lvm_lv_get_attr(const lv_t lv)
 	return lv_attr_dup(lv->vg->vgmem, lv);
 }
 
+const char *lvm_lv_get_origin(const lv_t lv)
+{
+	return lv_origin_dup(lv->vg->vgmem, lv);
+}
+
 struct lvm_property_value lvm_lv_get_property(const lv_t lv, const char *name)
 {
 	return get_property(NULL, NULL, lv, NULL, NULL, name);
-- 
1.8.1.4

[PATCH 2/2] python-lvm: Added lv method getOrigin

[Tony Asleson]

Signed-off-by: Tony Asleson <tasleson@redhat.com>
---
 python/liblvm.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/python/liblvm.c b/python/liblvm.c
index 96e94d4..09892ca 100644
--- a/python/liblvm.c
+++ b/python/liblvm.c
@@ -1202,6 +1202,14 @@ liblvm_lvm_lv_get_attr(lvobject *self)
 }
 
 static PyObject *
+liblvm_lvm_lv_get_origin(lvobject *self)
+{
+	LV_VALID(self);
+
+	return Py_BuildValue("s", lvm_lv_get_origin(self->lv));
+}
+
+static PyObject *
 liblvm_lvm_lv_get_name(lvobject *self)
 {
 	LV_VALID(self);
@@ -1758,6 +1766,7 @@ static PyMethodDef liblvm_lv_methods[] = {
 	/* lv methods */
 	{ "getAttr",		(PyCFunction)liblvm_lvm_lv_get_attr, METH_NOARGS },
 	{ "getName",		(PyCFunction)liblvm_lvm_lv_get_name, METH_NOARGS },
+	{ "getOrigin",		(PyCFunction)liblvm_lvm_lv_get_origin, METH_NOARGS },
 	{ "getUuid",		(PyCFunction)liblvm_lvm_lv_get_uuid, METH_NOARGS },
 	{ "activate",		(PyCFunction)liblvm_lvm_lv_activate, METH_NOARGS },
 	{ "deactivate",		(PyCFunction)liblvm_lvm_lv_deactivate, METH_NOARGS },
-- 
1.8.1.4

[PATCH 1/2] lvm2app: Add function to retrieve the origin.

[Zdenek Kabelac]

Dne 11.4.2013 22:58, Tony Asleson napsal(a):
> Signed-off-by: Tony Asleson <tasleson@redhat.com>
> ---
>   liblvm/lvm2app.h | 16 ++++++++++++++++
>   liblvm/lvm_lv.c  |  5 +++++
>   2 files changed, 21 insertions(+)
>
> diff --git a/liblvm/lvm2app.h b/liblvm/lvm2app.h
> index 15827ac..369c300 100644
> --- a/liblvm/lvm2app.h
> +++ b/liblvm/lvm2app.h
> @@ -1223,6 +1223,22 @@ const char *lvm_lv_get_name(const lv_t lv);
>   const char *lvm_lv_get_attr(const lv_t lv);
>
>   /**
> + * Get the origin of a snapshot.
> + *
> + * \memberof lv_t
> + *
> + * The memory allocated for the name is tied to the vg_t handle and will be
> + * released when lvm_vg_close() is called.
> + *
> + * \param   lv
> + * Logical volume handle.
> + *
> + * \return
> + * Null if the logical volume is not a snapshot, else origin name.
> + */
> +const char *lvm_lv_get_origin(const lv_t lv);
> +
> +/**
>    * Get the current size in bytes of a logical volume.
>    *
>    * \memberof lv_t
> diff --git a/liblvm/lvm_lv.c b/liblvm/lvm_lv.c
> index 9d3ae79..f244a60 100644
> --- a/liblvm/lvm_lv.c
> +++ b/liblvm/lvm_lv.c
> @@ -53,6 +53,11 @@ const char *lvm_lv_get_attr(const lv_t lv)
>   	return lv_attr_dup(lv->vg->vgmem, lv);
>   }
>
> +const char *lvm_lv_get_origin(const lv_t lv)
> +{
> +	return lv_origin_dup(lv->vg->vgmem, lv);
> +}
> +

If you need just 'const' pointer - it would be probably better,
to directly return the name - instead of duplication.

i.e. lv_origin_dup() ->  rewrite to  'const char *lv_get_origin(.)' ->
and use it for  lv_name_dup() in lv_origin_dup() which returns char *.
as well as for  lvm_lv_get_origin().



>   struct lvm_property_value lvm_lv_get_property(const lv_t lv, const char *name)
>   {
>   	return get_property(NULL, NULL, lv, NULL, NULL, name);
>


Zdenek

[PATCH 1/2] lvm2app: Add function to retrieve the origin.

[Tony Asleson]

On 04/12/2013 03:22 AM, Zdenek Kabelac wrote:
> If you need just 'const' pointer - it would be probably better,
> to directly return the name - instead of duplication.

Agreed.

> i.e. lv_origin_dup() ->  rewrite to  'const char *lv_get_origin(.)' ->
> and use it for  lv_name_dup() in lv_origin_dup() which returns char *.
> as well as for  lvm_lv_get_origin().

I ran into at least 2 different implementations of *get_origin.  There
is lv_origin_dup() and _origin_disp.  Both utilize the same logic with
slightly different output.  I haven't looked around it see if there are
others.

To clean this up correctly I would want to write what you are suggesting
and then use it in all the different places.

There is quite a bit of code that could certainly benefit from
decoupling the generation/manipulation of the data from the presentation
of it.

I've added this to a todo list.

Regards,
Tony

[PATCH 1/2] lvm2app: Add function to retrieve the origin.

[Zdenek Kabelac]

Dne 12.4.2013 17:44, Tony Asleson napsal(a):
> On 04/12/2013 03:22 AM, Zdenek Kabelac wrote:
>> If you need just 'const' pointer - it would be probably better,
>> to directly return the name - instead of duplication.
>
> Agreed.
>
>> i.e. lv_origin_dup() ->  rewrite to  'const char *lv_get_origin(.)' ->
>> and use it for  lv_name_dup() in lv_origin_dup() which returns char *.
>> as well as for  lvm_lv_get_origin().
>
> I ran into at least 2 different implementations of *get_origin.  There
> is lv_origin_dup() and _origin_disp.  Both utilize the same logic with
> slightly different output.  I haven't looked around it see if there are
> others.
>
> To clean this up correctly I would want to write what you are suggesting
> and then use it in all the different places.
>

Yep, that would be very useful - since we already do a tons of useless string 
duplication - we should try to avoid a new set of those ;)

There are many more places, where we could be happy with just const reference,
since the lifetime of object is clearly defined and parent object cannot be 
removed/changed, so there is no benefit of using duplicate copies.

Regards

Zdenek