[libvirt] [PATCH 1/1] storage: resize vol against real allocated size when --allocate is specified

[Wang Sen]

On Mon, Dec 16, 2013 at 01:18:05PM +0100, Michal Privoznik wrote:
> On 16.12.2013 06:00, Wang Sen wrote:
> > Currently, vol-resize allocates more bytes against vol->capacity, but
> > vol->capacity may be different from the real allocated size because --allocate
> > may not be specified. e.g.
> > 	[root at localhost ~]# virsh vol-list --pool tmp-pool --details
> > 	 name path type Capacity allocated
> > 	-------------------------------------------------------------
> > 	 tmp-vol  /root/tmp-pool/tmp-vol  file  1.00 GiB  1.00 GiB
> > 	[root at localhost ~]# virsh vol-resize tmp-vol --pool tmp-pool 2G
> > 	[root at localhost ~]# virsh vol-list --pool tmp-pool --details
> > 	 name path type Capacity allocated
> > 	-------------------------------------------------------------
> > 	 tmp-vol  /root/tmp-pool/tmp-vol  file  2.00 GiB  1.00 GiB
> > So, if we want to allocate more bytes to 3G, the real allocated size is 2G
> > actually.
> > 	[root at localhost ~]# virsh vol-resize tmp-vol --pool tmp-pool 3G --allocate
> > 	[root at localhost ~]# virsh vol-list --pool tmp-pool --details
> > 	 name path type Capacity allocated
> > 	-------------------------------------------------------------
> > 	 tmp-vol  /root/tmp-pool/tmp-vol  file  3.00 GiB  2.00 GiB
> > This patch enable resize vol against the real allocated size. After this patch
> > is applied, the result of the last resize command become 3G.
> > 	[root at localhost ~]# virsh vol-resize tmp-vol --pool tmp-pool 3G --allocate
> > 	[root at localhost ~]# virsh vol-list --pool tmp-pool --details
> > 	 name path type Capacity allocated
> > 	-------------------------------------------------------------
> > 	 tmp-vol  /root/tmp-pool/tmp-vol  file  3.00 GiB  3.00 GiB
> > 
> > Signed-off-by: Wang Sen <wangsen at linux.vnet.ibm.com>
> > ---
> >  src/storage/storage_backend_fs.c | 17 ++++++++++++++++-
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
> > index 11cf2df..e75c57b 100644
> > --- a/src/storage/storage_backend_fs.c
> > +++ b/src/storage/storage_backend_fs.c
> > @@ -1261,13 +1261,28 @@ virStorageBackendFileSystemVolResize(virConnectPtr conn ATTRIBUTE_UNUSED,
> >                                       unsigned long long capacity,
> >                                       unsigned int flags)
> >  {
> > +    int fd = -1;
> >      virCheckFlags(VIR_STORAGE_VOL_RESIZE_ALLOCATE, -1);
> > +    unsigned long long orig_capacity;
> > +    virStorageFileMetadataPtr meta = NULL;
> > +
> > +    if ((fd = open(vol->target.path, O_RDWR)) < 0) {
> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Can not open file"));
> > +    }
> > +
> > +    if (!(meta =  virStorageFileGetMetadataFromFD(vol->target.path, fd, \
> > +                                                  vol->target.format))) {
> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Get meta data error!"));
> > +    }
> > +    orig_capacity = meta->capacity;
> > +    virStorageFileFreeMetadata(meta);
> > +    close(fd);
> 
> Using close() is prohibited and you should you VIR_CLOSE instead.
> However, there's no need to do this while we have
> virStorageFileGetMetadata(), Nor it's needed for other formats than _RAW.
> 
> >  
> >      bool pre_allocate = flags & VIR_STORAGE_VOL_RESIZE_ALLOCATE;
> >  
> >      if (vol->target.format == VIR_STORAGE_FILE_RAW) {
> >          return virStorageFileResize(vol->target.path, capacity,
> > -                                    vol->capacity, pre_allocate);
> > +                                    orig_capacity, pre_allocate);
> >      } else {
> >          if (pre_allocate) {
> >              virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> > 
> 
> I agree that we have a bug here. But I think we need this modified
> version. I'll post it shortly.

okay, thanks a lot.

> 
> Michal
>