[libvirt] [PATCH v2 4/6] vol: Add thin_pool to _virStorageVolSource

[Pavel Hrdina]

On Mon, Feb 01, 2016 at 09:00:51AM -0500, John Ferlan wrote:
> 
> 
> On 02/01/2016 07:45 AM, Pavel Hrdina wrote:
> > On Thu, Jan 28, 2016 at 05:44:07PM -0500, John Ferlan wrote:
> >> A thin lv doesn't have any extents defined - rather it uses a concept of
> >> a "thin-pool" in order to describe it's source. In order to allow that to
> >> be displayed properly in a future patch, add a new 'thin_pool' name that
> >> can be used by a future patch to store the name of the source thin_pool
> >> name used by a thin logical volume.
> >>
> >> Signed-off-by: John Ferlan <jferlan at redhat.com>
> >> ---
> >>  src/conf/storage_conf.c | 5 +++++
> >>  src/conf/storage_conf.h | 1 +
> >>  2 files changed, 6 insertions(+)
> >>
> >> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> >> index 3657dfd..8ceb465 100644
> >> --- a/src/conf/storage_conf.c
> >> +++ b/src/conf/storage_conf.c
> >> @@ -322,6 +322,7 @@ virStorageVolDefFree(virStorageVolDefPtr def)
> >>      for (i = 0; i < def->source.nextent; i++)
> >>          VIR_FREE(def->source.extents[i].path);
> >>      VIR_FREE(def->source.extents);
> >> +    VIR_FREE(def->source.thin_pool);
> >>  
> >>      virStorageSourceClear(&def->target);
> >>      VIR_FREE(def);
> >> @@ -1655,6 +1656,10 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
> >>              virBufferAddLit(&buf, "</device>\n");
> >>      }
> >>  
> >> +    if (def->source.thin_pool)
> >> +        virBufferEscapeString(&buf, "<name>%s</name>\n",
> >> +                              def->source.thin_pool);
> > 
> > I'm not sure about the element to be called "name".  I would suggest something
> > like <thinpool name=''/>.  Otherwise it looks good.
> 
> I used <name> because it would be hierarchically similar to
> <pool>...<source>...<name>... That is <volume
> type='block'>...<source>...<name>
> 
> Although this does remind me, storagevol.rng needs to be updated seeing
> as <device path='%s'> <extent start=### end=###/> is defined.
> 
> Alternatively, would the following work?
> 
> <volume type='block'>
> ...
>   <source>
>     <name>%s</name>
>     <format type='thin_pool'>
>   </source>
> ...
> </volume>
> 
> Trying to forward think if/when a vol.xml was used as input...

Maybe we should stop right here and handle the thin pool/volumes in a separate
patch series.  It would be best to create a new libvirt pool type that will be
dedicated for lvm thin pools and add a proper support for thin pools.

Pavel

> 
> John
> > 
> > Pavel
> > 
> >> +
> >>      virBufferAdjustIndent(&buf, -2);
> >>      virBufferAddLit(&buf, "</source>\n");
> >>  
> >> diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
> >> index f1dc62b..7e15a70 100644
> >> --- a/src/conf/storage_conf.h
> >> +++ b/src/conf/storage_conf.h
> >> @@ -55,6 +55,7 @@ struct _virStorageVolSource {
> >>  
> >>      int partType; /* virStorageVolTypeDisk, only used by disk
> >>                     * backend for partition type creation */
> >> +    char *thin_pool; /* Used to print/dumpxml the thin pool name */
> >>  };
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list