cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marcus Sorensen <>
Subject Re: Review Request: resize volume initial implementation
Date Mon, 17 Sep 2012 15:42:43 GMT
---------- Forwarded message ----------
From: Marcus Sorensen <>
Date: Fri, Sep 14, 2012 at 8:17 AM
Subject: RE: Review Request: resize volume initial implementation
To: Koushik Das <>

Thanks! Replies below.

On Sep 14, 2012 4:12 AM, "Koushik Das" <> wrote:
> Some comments inline.
> -----Original Message-----
> From: Marcus Sorensen [] On Behalf Of Marcus Sorensen
> Sent: Friday, September 14, 2012 6:09 AM
> To: Marcus Sorensen; cloudstack
> Subject: Review Request: resize volume initial implementation
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> -----------------------------------------------------------
> Review request for cloudstack.
> Description
> -------
> Initial implementation of resize volumes. Works only for KVM but can be easily used to
add in other hypervisors. Works with local,sharedmountpoint,NFS,and CLVM storage. This is
a significant chunk of code and my first attempt at a new API call so please let me know if
there are any issues with where/how things are done.
> This KVM implementation includes a host script "" because of several current
limitations. 1) we don't seem to have java bindings for virStorageVolResize() or virDomainBlockResize(),
and even if we did, virStorageVolResize() doesn't work for logical volume pools. It will presumably
be awhile before that's patched and available on current distros.
> New API call is 'resizeVolume', with parameters:
> 'id' for volume id
> 'size' for new size, accepts things like 10G, 10240M, 10485760K, 10737418240B or 10737418240
> [Koushik] Should this be the delta instead of the new size?

I like working in absolutes personally. A developer could make their
interface present deltas to the user. Actually there's no reason not
to accept either, I could do + for grow, - for shrink, and neither for

> 'shrinkok' this one is a boolean that confirms the user is ok with the volume shrinking.
I did this because it seems reasonable that someone might want to give back storage, and since
it's potentially dangerous (users need to free up the end of the block device that they want
to give back) there needs to be some sort of signoff. This can be disabled/removed if others
think it's too much of a liability. The code checks size twice, comparing the requested size
once against what we think the volume size is per database, and once again comparing the actual
qcow2/lv size against the requested size, both times ensuring that shrinkok is true before
> [Koushik] I think this should be provided only if the volume is usable after shrinking
it. Also rather than asking user to free/compact data it will be good if CS does the same
using some tool.

Both the qcow2 and lvm (raw) are usable after shrink, provided the
necessary precautions are taken to evacuate the end of the device. I
think it would be fairly difficult to free up the space on our own as
the user could do any number of things with a volume.

> If the resize succeeds, but libvirt fails to live update qemu of the new size (whether
due to bug, non-virtio disks, or something else), there's currently no indication other than
that the API call returns the new size as seen from libvirt, which itself should be an indication
that a powercycle is needed if the API call succeeds, the size is what was requested, and
the host isn't seeing the new size. Perhaps a field should be added, like 'rebootrequired:true'
to make it easy.
> One thing I haven't tackled at all is how to handle the service offering fields.  If
someone has a service offering with a static 5GB discription like the default storage offerings
have, that won't change. Suggestions welcome. Should we update the service offering to custom,
or could that mess up other things like tags?
> [Koushik] Some compute/disk offering can be created with a range for disk size (low value,
high value). As long as the resize doesn't result in going out of range, it should be allowed.

So you're saying that only certain disk offerings should allow resize?
We'd need to add properties to the disk offerings, but that should be
doable. I think it would cause problems though because few people
think much about wanting to resize initially. I know for us it's
mainly driven by wanting variable sized root disks (no service
offering, correct?) and small templates.

> Diffs
> -----
>   api/src/com/cloud/agent/api/storage/ e69de29
>   api/src/com/cloud/agent/api/storage/ e69de29
>   api/src/com/cloud/api/ 067ddf7
>   api/src/com/cloud/api/commands/ e69de29
>   api/src/com/cloud/event/ e84a403
>   api/src/com/cloud/storage/ 4fb3b55
>   api/src/com/cloud/storage/ 6e8e48e
>   client/tomcatconf/ e233694
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/
>   scripts/storage/qcow2/ e69de29
>   server/src/com/cloud/storage/ 83b2846
> Diff:
> Testing
> -------
> Tested CLVM,NFS,local,sharedmountpoint, qcow2 and lvm formats
> create test volumes on above listed pools, attach to VM instance
> within instance, format as ext4, populate with files, grow, resize filesystem: pass
> within instance, format as ext4, populate with files, shrink filesystem, shrink volume,
unmount, fsck, remount: pass
> try passing bad arguments to API call fails as expected
> try resizing as wrong user fails as expected
> force to fail through various means bubbles the error up as expected,
resets the volume state to Ready
> Thanks,
> Marcus Sorensen

View raw message