Hi Scott, Yes, What you said true || false = true haha ;)
yeah, had me a bit confused why this wasn't working for a moment, but the checks on quota will fail to produce the correct results when partiular combinations of boolean values are present. For example, looking at the can_create_instance method. if Quota.no_limit(maximum_total_instances) || maximum_total_instances >= potential_total_instances && Quota.no_limit(maximum_total_storage) || maximum_total_storage.to_f >= potential_total_storage.to_f Works fine unless you have these combination: "flase || false && false || true" or "true || false && false || flase" In these examples, the correct behaviour is for the method to return false, but in fact it returns true, Anyways... I've made the changes and it seems to work, I can resubmit the patches with changes or I could push with the changes if your happy with that :-) Cheers Martyn ----- Original Message ----- From: "Scott Seago" <[email protected]> To: [email protected] Cc: [email protected] Sent: Monday, June 21, 2010 5:42:09 AM GMT +00:00 GMT Britain, Ireland, Portugal Subject: Re: [deltacloud-devel] [PATCH aggregator 1/2] Added Ability to Specify No Limits on Quota [email protected] wrote: > From: martyntaylor <[email protected]> > > --- > src/app/controllers/quota_controller.rb | 73 +++++++---------------- > src/app/models/quota.rb | 60 +++++++++++-------- > src/db/migrate/20090802000000_create_quotas.rb | 10 ++-- > 3 files changed, 62 insertions(+), 81 deletions(-) > > This was almost working for me. The UI for getting/setting seems to be fine, but there seem to be some problems around quota enforcement. > diff --git a/src/app/models/quota.rb b/src/app/models/quota.rb > index 43ab9a2..324a384 100644 > --- a/src/app/models/quota.rb > +++ b/src/app/models/quota.rb > @@ -20,33 +20,24 @@ > # Likewise, all the methods added will be available for all controllers. > > class Quota < ActiveRecord::Base > - has_one :pool > - has_one :cloud_account > - > - validates_presence_of :maximum_running_instances > - validates_presence_of :maximum_running_memory > - validates_presence_of :maximum_running_cpus > > - validates_presence_of :maximum_total_storage > - validates_presence_of :maximum_total_instances > + QuotaResource = Struct.new(:name, :used, :max, :available, :unit) > > - validates_numericality_of :maximum_running_instances > - validates_numericality_of :maximum_running_memory > - validates_numericality_of :maximum_running_cpus > - > - validates_numericality_of :maximum_total_storage > - validates_numericality_of :maximum_total_instances > + NO_LIMIT = nil > > + has_one :pool > + has_one :cloud_account > > def can_create_instance?(instance) > - # TODO Fix: When this returns failed, instance gets deleted at some > point from database. It should be kept for audit > hwp = instance.hardware_profile > > potential_total_storage = total_storage.to_f + hwp.storage.value.to_f > potential_total_instances = total_instances + 1 > > - if maximum_total_instances >= potential_total_instances && > maximum_total_storage.to_f >= potential_total_storage.to_f > - return true > + # check for no quota > + if Quota.no_limit(maximum_total_instances) || maximum_total_instances >= > potential_total_instances && > + Quota.no_limit(maximum_total_storage) || maximum_total_storage.to_f > >= potential_total_storage.to_f > You will need some parentheses to get the desired logic above: if (Quota.no_limit(maximum_total_instances) || maximum_total_instances >= potential_total_instances) && (Quota.no_limit(maximum_total_storage) || maximum_total_storage.to_f >= potential_total_storage.to_f) > + return true > end > return false > end > @@ -58,17 +49,36 @@ class Quota < ActiveRecord::Base > potential_running_memory = running_memory.to_f + hwp.memory.value.to_f > potential_running_cpus = running_cpus.to_f + hwp.cpu.value.to_f > > - if maximum_running_instances >= potential_running_instances && > maximum_running_memory.to_f >= potential_running_memory && > maximum_running_cpus.to_f >= potential_running_cpus > - return true > + if Quota.no_limit(maximum_running_instances) || > maximum_running_instances >= potential_running_instances && > + Quota.no_limit(maximum_running_memory) || maximum_running_memory.to_f > >= potential_running_memory && > + Quota.no_limit(maximum_running_cpus) || maximum_running_cpus.to_f >= > potential_running_cpus > + return true > Similarly you'll need the parentheses here -- I think the above is where it was failing for me, as if you have only some of the limits set, incorrect order of combining the above and/or bits will result in the wrong results: if (Quota.no_limit(maximum_running_instances) || maximum_running_instances >= potential_running_instances) && (Quota.no_limit(maximum_running_memory) || maximum_running_memory.to_f >= potential_running_memory) && (Quota.no_limit(maximum_running_cpus) || maximum_running_cpus.to_f >= potential_running_cpus) return true I think these two patches will be good to go once this is fixed, but I'll give it another quick test tomorrow. Scott _______________________________________________ deltacloud-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/deltacloud-devel
