On 02/01/12 14:03, Michal Fojtik wrote:
> Hi,
>
> ACK.
thanks for review
>
> Just one question/idea, not related to this patch.
>
> Wouldn't be cleaner to rather than creating a 'special' operation to add
> 'update' operation
> that will change the 'instance_id' in storage_volume resource?
>
yeah... in CIMI, attaching a volume is an operation of Machine - you
attach the volume to a Machine, by PUTting the Machine description
including the volume(s) you want attaching (or omitting the ones you
want to detach). For now we have not implemented this in this way,
instead using a 'special' attach_volume operation of Machine. It is not
intended to be permanent, but just something for us to use as a starting
point before implementing eventually the 'proper' way (parsing the
passed in Machine description and picking apart all the components of
the Machine, in this case the Volumes).
> I mean something like:
>
> PUT /api/storage_volumes/vol1
>
> Where the params will look like:
>
> { 'instance_id' => 'inst1' }
>
> For me it sounds more 'natural' that we want to 'PUT' (literally) an instance
> to this storage_volume.
OK, for me it sounds more natural to PUT the volume to the instance (I
think that's what you suggest below though so not sure if I understood
the above correctly). In any case though, the fact remains that in CIMI,
attaching a Volume is an operation on a Machine, not on the Volume (in
contrast, Deltacloud 'attach_instance' is an operation of StorageVolume).
Or _even_ 'more' natural will be that we will 'PUT' the
> storage_volume to instance. In that case the 'instances' collection will
> define:
>
> collection :instances do
> operation :volumes, :method => :put, :member => true do
> end
> end
>
> Then client will do:
>
> PUT /api/instances/inst1/volumes { 'storage_volume_id' => 'vol1' }
THis is how the current (temporary) CIMI attach_volume works:
PUT /cimi/Machines/machine1/attach_volume {
-H "Content-Type: application/CIMI-Machine+xml"
-d '<VolumeAttach><volume
href="http://localhost:3001/cimi/volumes/vol-019dfd6c";
attachmentPoint="/dev/sdc"/></VolumeAttach>'}
>
> Then we somehow need to list the volumes in 'instance'. Just my 50c ;-)
>
yes, this is why I added the 'storage_volumes' attribute to Deltacloud
Instance model - luckily EC2 API will return this attribute for
description of an Instance, so you get the volume_id and its device
(e.g. /dev/sdc) for any attached volumes
marios
> -- Michal
>
> On Dec 23, 2011, at 5:31 PM, [email protected] wrote:
>
>> From: marios <[email protected]>
>>
>>
>> Signed-off-by: marios <[email protected]>
>> ---
>> server/lib/deltacloud/server.rb | 10 ++++++++++
>> server/views/storage_volumes/show.html.haml | 2 +-
>> 2 files changed, 11 insertions(+), 1 deletions(-)
>>
>> diff --git a/server/lib/deltacloud/server.rb
>> b/server/lib/deltacloud/server.rb
>> index 88250a6..2fa81c7 100644
>> --- a/server/lib/deltacloud/server.rb
>> +++ b/server/lib/deltacloud/server.rb
>> @@ -664,6 +664,16 @@ collection :storage_volumes do
>> end
>> end
>>
>> + operation :attach_instance, :method=>:get, :member=>true do
>> + description "A form to attach a storage volume to an instance"
>> + control do
>> + @instances = driver.instances(credentials)
>> + respond_to do |format|
>> + format.html{ haml :"storage_volumes/attach"}
>> + end
>> + end
>> + end
>> +
>> operation :attach, :method => :post, :member => true do
>> description "Attach storage volume to instance"
>> with_capability :attach_storage_volume
>> diff --git a/server/views/storage_volumes/show.html.haml
>> b/server/views/storage_volumes/show.html.haml
>> index 585990e..aabdd85 100644
>> --- a/server/views/storage_volumes/show.html.haml
>> +++ b/server/views/storage_volumes/show.html.haml
>> @@ -32,6 +32,6 @@
>> =link_to_action "Snapshot",
>> api_url_for("storage_snapshots/new?volume_id=#{@storage_volume.id}"), :get
>> - unless @storage_volume.instance_id
>> =link_to_action "Delete",
>> destroy_storage_volume_url(@storage_volume.id), :delete
>> - =link_to_action "Attach",
>> api_url_for("storage_volumes/attach?id=#{@storage_volume.id}"), :get
>> + =link_to_action "Attach",
>> api_url_for("storage_volumes/#{@storage_volume.id}/attach_instance"), :get
>
> I think, you can also use:
>
> attach_instance
>
>> - if @storage_volume.instance_id
>> =link_to_action "Detach",
>> detach_storage_volume_url(@storage_volume.id), :post
>> --
>> 1.7.6.4
>>
>
> ------------------------------------------------------
> Michal Fojtik, [email protected]
> Deltacloud API: http://deltacloud.org
>
