> -----Original Message----- > From: Marcus Sorensen [mailto:shadow...@gmail.com] > Sent: Friday, September 21, 2012 4:47 PM > To: cloudstack-dev@incubator.apache.org > Subject: Re: Review Request: resize volume initial implementation > > When you create an offering, you can select 'custom disk size' in the > offering, allowing the user to specify a size. It's possible that the > data disk was created with custom offering, so this particular > offering is not bound to a particular size and it would seem that > we're free to resize without switching between offerings. They also > may be trying to go between a custom and a static sized offering.
As long as we don't directly use diskoffering.getsize() as the volume size in the code(there is only one place storagemanagerImpl->movevolume), it's ok for me. > > On Fri, Sep 21, 2012 at 5:23 PM, Edison Su <edison...@citrix.com> wrote: > > > > > >> -----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 > >> >>>>>>>
