How about root volumes, maybe just leave those for root admin to be able to resize? Or leave them off altogether?
On Thu, Sep 20, 2012 at 8:07 PM, Marcus Sorensen <shadow...@gmail.com> wrote: > OK, that has me thinking... so the existing implementation can be valid for > offerings of custom size (the only time you would pass a size). > > Otherwise you just pass the resize API call a new disk offering, and we make > sure the tags are the same. This way people are still in control of which > offerings are available and users can only resize between what is offered. > But they will be required to create any offerings themselves. > > On Sep 20, 2012 7:50 PM, "Edison Su" <edison...@citrix.com> wrote: >> >> >> >> > -----Original Message----- >> > From: Marcus Sorensen [mailto:shadow...@gmail.com] >> > Sent: Monday, September 17, 2012 8:54 AM >> > To: cloudstack-dev@incubator.apache.org >> > Subject: Re: Review Request: resize volume initial implementation >> > >> > Just forwarded this because I realized I didn't reply all. >> > >> > We can create a new property for service offerings, a 'resizable' >> > checkbox. That will likely require a database schema change, and I'll >> > need some help with that. >> > >> > Alternatively, we could check and only allow resizing on disk >> > offerings that are of custom size. >> > >> > These will sort of suck for people who haven't planned well, but it >> > keeps the resize feature from breaking what seems to be part of the >> > point of the disk service offerings, which is to allow people to offer >> > and bill for packaged storage sizes. >> >> How about use the following api call to change size of volume: >> 1. create a new disk offering with a new size >> 2. upgrade disk offering of a volume with the new disk offering. If the >> disk size is different, then resize the volume. >> >> > >> > >> > >> > On Mon, Sep 17, 2012 at 9:42 AM, Marcus Sorensen <shadow...@gmail.com> >> > wrote: >> > > ---------- Forwarded message ---------- >> > > From: Marcus Sorensen <shadow...@gmail.com> >> > > Date: Fri, Sep 14, 2012 at 8:17 AM >> > > Subject: RE: Review Request: resize volume initial implementation >> > > To: Koushik Das <koushik....@citrix.com> >> > > >> > > >> > > Thanks! Replies below. >> > > >> > > On Sep 14, 2012 4:12 AM, "Koushik Das" <koushik....@citrix.com> wrote: >> > >> >> > >> Some comments inline. >> > >> >> > >> -----Original Message----- >> > >> From: Marcus Sorensen [mailto:nore...@reviews.apache.org] 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: >> > >> https://reviews.apache.org/r/7099/ >> > >> ----------------------------------------------------------- >> > >> >> > >> 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 "resizevolume.sh" >> > 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 >> > > absolute. >> > > >> > >> >> > >> '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 continuing. >> > >> [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/ResizeVolumeAnswer.java >> > e69de29 >> > >> api/src/com/cloud/agent/api/storage/ResizeVolumeCommand.java >> > e69de29 >> > >> api/src/com/cloud/api/ApiConstants.java 067ddf7 >> > >> api/src/com/cloud/api/commands/ResizeVolumeCmd.java e69de29 >> > >> api/src/com/cloud/event/EventTypes.java e84a403 >> > >> api/src/com/cloud/storage/StorageService.java 4fb3b55 >> > >> api/src/com/cloud/storage/Volume.java 6e8e48e >> > >> client/tomcatconf/commands.properties.in e233694 >> > >> >> > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtCo >> > mputingResource.java 9312519 >> > >> scripts/storage/qcow2/resizevolume.sh e69de29 >> > >> server/src/com/cloud/storage/StorageManagerImpl.java 83b2846 >> > >> >> > >> Diff: https://reviews.apache.org/r/7099/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 resizevolume.sh to fail through various means bubbles the >> > error up as expected, resets the volume state to Ready >> > >> >> > >> >> > >> Thanks, >> > >> >> > >> Marcus Sorensen >> > >>
