incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Edison Su <Edison...@citrix.com>
Subject RE: Review Request: resize volume initial implementation
Date Fri, 21 Sep 2012 23:23:39 GMT


> -----Original Message-----
> From: Marcus Sorensen [mailto:shadowsor@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.su@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" <shadowsor@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
> <shadowsor@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.su@citrix.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Marcus Sorensen [mailto:shadowsor@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
> <shadowsor@gmail.com>
> >>>>> wrote:
> >>>>>> ---------- Forwarded message ----------
> >>>>>> From: Marcus Sorensen <shadowsor@gmail.com>
> >>>>>> Date: Fri, Sep 14, 2012 at 8:17 AM
> >>>>>> Subject: RE: Review Request: resize volume initial
> implementation
> >>>>>> To: Koushik Das <koushik.das@citrix.com>
> >>>>>>
> >>>>>>
> >>>>>> Thanks! Replies below.
> >>>>>>
> >>>>>> On Sep 14, 2012 4:12 AM, "Koushik Das" <koushik.das@citrix.com>
> wrote:
> >>>>>>>
> >>>>>>> Some comments inline.
> >>>>>>>
> >>>>>>> -----Original Message-----
> >>>>>>> From: Marcus Sorensen [mailto:noreply@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
> >>>>>>>

Mime
View raw message