incubator-deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eric Woods <>
Subject Re: IBM SBC Driver Available
Date Wed, 02 Feb 2011 20:30:07 GMT
Thanks, David.  See responses below.

Unfortunately I'm not able to provide a test account for IBM's cloud, so we'll have to "fake
it" for test cases.  Is writing/running tests documented anywhere?

- Eric W.

On Feb 1, 2011, at 6:26 PM, David Lutterkort wrote:

> On Mon, 2011-01-31 at 14:37 -0500, Eric Woods wrote:
>> I've updated the driver based on David's feedback and uploaded a
>> patch:  I'm new to
>> ruby, so this feedback is really appreciated.
> That looks good now, with the one exception of the @last_image business
> (see below)
>> Some notes:
>> 2) I maintained an @last_image field to avoid an extra image lookup
>> for performance reasons.  I made this more robust by checking for nil
>> and performing a lookup if necessary.
> Still don't like it, since it makes the default realm and HWP hard to
> predict. You might get used to the fact that you'll always get HWP x
> when you create an image without specifying the HWP explicitly, but then
> one day your code changes, and suddenly you get HWP y.
> Since we already enumerate all the HWP explicitly, just pick one as the
> default (I assume COP32.1/2048/60 would be the simplest) As for realm,
> just hardcode one fairly arbitrary one.
> These choices at least make the defaults for create instance predictable
> (and documentable ;)

The issue is not all hardware profiles are valid for all images.  For each image, a list of
valid HWP configurations are provided.  This mandates that I check the corresponding image
to determine a valid "default."  I noticed that images() is called when a create request is
issued, so I stored the image there rather than performing an additional lookup later.  I
recognize that this is a workaround, but I'm not sure how to pick a default when valid configurations
are on a per-instance-basis.  Please advise.

>> 3) The online documentation states that nothing is returned from
>> reboot/stop/destroy.  We should update the documentation to indicate
>> an instance is returned for each of these.
> Where do you see that ? Definitely needs to be fixed.  Near the bottom where reboot_instance()
and stop_instance() are documented.

>> 4) For instances(credentials, opts={}), this method/function is being
>> passed opts=nil which causes an exception, so I kept the line opts ||=
>> {}
> Ok .. works for me.
> David

View raw message