deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Koper, Dies" <di...@fast.au.fujitsu.com>
Subject RE: [PATCH] FGCP: added support for storage_volume creation from snapshot
Date Sat, 23 Feb 2013 02:11:45 GMT
> ACK; though there are a couple of things that need improving: (1) the
> fact that there are no unit tests and (2) spawning a new thread for the
> snapshot creation:

I discussed (1) with mfojtik on irc.
The problem is that I'm polling the server until the volume's state changes from CREATING
to AVAILABLE.
So the request is the same, but the response at some point changes. Not sure if VCR can do
that.
But I haven't investigated either.
The fact that polling happens in a separate thread may complicate things more. (or actually
make it easier, as any errors will have to be ignored anyway)
As I increase test coverage for fgcp, I'd like to revisit this.

> One question about this is more an FGCP API question: since snapshot
> creation is so heavily asynchronous, snapshots may be very different
> from the state the disk was in when the snapshot request was made.

In FGCP, snapshots can only be created from volumes that are not attached or attached to an
instance in the STOPPED status.
I believe an instance can be started soon after the initiation of the backup has started,
but the snapshot will be based on the state before starting.

> The other issue is more serious: this makes it possible for users to
> spawn a thread on every request, and is likely a way to trigger a DOS.

I don't think that's easy to do with this operation:
I do as many backend calls as possible before creating the thread (even run the restore operation
once before creating the thread) to ensure all resources are in the right state.
So simply running the same operation with the same snapshot will immediately return an error
saying that the request cannot be accepted because a volume is being created, or because the
snapshot is in use for restoring.
But I get your point.

Cheers,
Dies Koper

> -----Original Message-----
> From: David Lutterkort [mailto:lutter@redhat.com]
> Sent: Saturday, 23 February 2013 11:06 AM
> To: dev@deltacloud.apache.org
> Subject: Re: [PATCH] FGCP: added support for storage_volume creation from
> snapshot
> 
> On Fri, 2013-02-22 at 12:11 +1100, diesk@fast.au.fujitsu.com wrote:
> > From: Dies Koper <diesk@fast.au.fujitsu.com>
> >
> > ---
> >  server/lib/deltacloud/drivers/fgcp/fgcp_client.rb | 44
> +++++++++++++++++++++++
> >  server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb | 38
> +++++++++++++++++++-
> >  server/views/storage_volumes/show.html.haml       |  2 +-
> >  3 files changed, 82 insertions(+), 2 deletions(-)
> 
> ACK; though there are a couple of things that need improving: (1) the
> fact that there are no unit tests and (2) spawning a new thread for the
> snapshot creation:
> 
> > diff --git a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> > index 71b53bd..8d29a8e 100644
> > --- a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> > +++ b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> > @@ -556,7 +556,43 @@ class FgcpDriver < Deltacloud::BaseDriver
> > +        #wait until creation completes in a separate thread
> > +        Thread.new {
> > +          attempts = 0
> > +          begin
> > +            sleep 10
> > +            # this fails if the destination vdisk is still being
> deployed
> > +            client.external_restore_vdisk(orig_vsys_id, backup_id,
> opts[:realm_id], vdisk_id, key)
> 
> One question about this is more an FGCP API question: since snapshot
> creation is so heavily asynchronous, snapshots may be very different
> from the state the disk was in when the snapshot request was made.
> 
> The other issue is more serious: this makes it possible for users to
> spawn a thread on every request, and is likely a way to trigger a DOS.
> It would be _much_ preferrable to stick this into a work queue - it
> wouldn't be all that hard to pull in DelayedJob for this purpose. I'd
> be
> fine with adding a table to the DB to store these jobs.
> 
> David
> 
> 

Mime
View raw message