> -----Original Message----- > From: Marcus Sorensen [mailto:shadow...@gmail.com] > Sent: Friday, September 21, 2012 8:34 AM > To: cloudstack-dev@incubator.apache.org > Subject: Re: Review Request: resize volume initial implementation > > Yeah, that was the point. If people are charging a price based on disk > offering, then we don't want users creating any disk offering they > want. Isn't that the reason for disk offering? > > I guess if root admin can only create disk offerings, then the > proposal is correct, since we don't want to let users get around that > by resize creating a disk offering. In the end users should only ever > be able to have disks of sizes that the admin offers and should not be > able to define offerings. > > So here is my new proposal logic: > > * Only resize data disks > > * resize based on an offering id, or size if custom offering
What's the "custom offering" mean? Variable size? To make it simple, how about just based on an offering id? > > * if current offering is custom sized, allow resize by simply passing a > size > > * ok to migrate from custom offering to static size offering based on > offerings available > > * ok to migrate from static size offering to custom size offering > based on offerings available > > * tags must match between old/new offerings > > * if migrating to a custom size, size MUST be passed > > On Fri, Sep 21, 2012 at 1:16 AM, Edison Su <edison...@citrix.com> wrote: > > root admin defines the disk offerings that user can use. So user can > only upgrade or downgrade disk offering to whatever admin defines. It > is not flexible enough, but seems working. > > Only resize data disk is enough, I think. It's easy to recreate a > root disk than a data disk(copying a data on a big data disk to another > place takes time, recreate a root disk is fast). If this assumption is > valid, we can conclude that user may not want to recreate data disk > just because current data disk offering is too large or small. > > My 0.2$ > > > > Sent from my iPhone > > > > On Sep 20, 2012, at 8:57 PM, "Marcus Sorensen" <shadow...@gmail.com> > wrote: > > > >> 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 > >>>>>>>
