Not sure I understand. I would use that to fetch the new size for moving to a disk offering of a static size (else how would I know what size to migrate to), but for a custom sized offering I'd require a size parameter be specified. Or is that call unreliable or something else wrong with it? On Sep 21, 2012 7:07 PM, "Edison Su" <edison...@citrix.com> wrote:
> > -----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 > > >> >>>>>>> >
