OpenStack Compute (nova)

Merge lp:~jordanrinke/nova/lp707171 into lp:~hudson-openstack/nova/trunk

  1. lp707171
  2. Merge into trunk
Proposed by Jordan Rinke
Status: Merged
Approved by: Vish Ishaya
Approved revision: 612
Merged at revision: 631
Proposed branch: lp:~jordanrinke/nova/lp707171
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 24 lines (+2/-1)
2 files modified
Authors (+1/-0)
nova/virt/hyperv.py (+1/-1)
To merge this branch: bzr merge lp:~jordanrinke/nova/lp707171
Reviewer Review Type Date Requested Status
Jay Pipes (community) Approve
Thierry Carrez (community) Approve
Devin Carlen (community) Approve
Review via email: mp+47325@code.launchpad.net

Description of the change

Changed cpu limit to a static value of 100000 (100%) instead of using the vcpu value of 1. There is no weight/limit variable now so I see no other solution than the static max limit.

To post a comment you must log in.
Revision history for this message
Devin Carlen (devcamcar) wrote :

lgtm

review: Approve
Revision history for this message
Thierry Carrez (ttx) wrote :

lgtm

review: Approve
Revision history for this message
Soren Hansen (soren) wrote :

What does 100% mean? Does that mean "one CPU" or does it mean "as many CPU's as there are on the host"?

Revision history for this message
Jordan Rinke (jordanrinke) wrote :

100% means potentially complete usage of the number of cores allocated/reserved. So, if the vcpu value is 1, hyper-v allocates 1 virtual cpu, and reserves 1 core on the system for that vcpu, the 100% limit allows it to use that entire reserved amount. (it is kind of a duplicate cap, I don't know why one would reserve CPU cycles and then limit anything below that since you have already taken the resource)

I am not sure how that works on libvirt / what the settings are right now, we might want to adjust the reservation amount to mirror that, since right now it is an entire core per vcpu with hyper-v...

Revision history for this message
Jay Pipes (jaypipes) wrote :

Hi Jordan!

Code is fine, but warning: you'll hit a pep8 issue. You need to put 2 spaces before an inline-comment, so this:

procsetting.Limit = 100000 # static assignment to 100% since no limit/weight variable exists now.

needs to be this:

procsetting.Limit = 100000 # static assignment to 100% since no limit/weight variable exists now.

I know, silly, but it'll prevent the merge from going through :) Run ./run_tests.sh -V to run all tests and run pep8 against source files. You should see the error pop up.

Cheers!
jay

review: Needs Fixing
Revision history for this message
Jordan Rinke (jordanrinke) wrote :

Fixed.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

The person who submitted this branch (jordanrinke) does not appear in http://wiki.openstack.org/Contributors?action=raw

Revision history for this message
Thierry Carrez (ttx) wrote :

Cool beans, someone got caught by my CLA verifier !
Jordan: please sign CLA and add yourself to http://wiki.openstack.org/Contributors

Revision history for this message
Jordan Rinke (jordanrinke) wrote :

Done.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :
Download full text (20.5 KiB)

The attempt to merge lp:~jordanrinke/nova/lp707171 into lp:nova failed. Below is the output from the failed tests.

AdminAPITest
    test_admin_disabled ok
    test_admin_enabled ok
APITest
    test_exceptions_are_converted_to_faults ok
Test
    test_authorize_token ok
    test_authorize_user ok
    test_bad_token ok
    test_bad_user ok
    test_no_user ok
    test_token_expiry ok
TestLimiter
    test_authorize_token ok
TestFaults
    test_fault_parts ok
    test_raise ok
    test_retry_header ok
FlavorsTest
    test_get_flavor_by_id ok
    test_get_flavor_list ok
GlanceImageServiceTest
    test_create ok
    test_create_and_show_non_existing_image ok
    test_delete ok
    test_update ok
ImageControllerWithGlanceServiceTest
    test_get_image_details ok
    test_get_image_index ok
LocalImageServiceTest
    test_create ok
    test_create_and_show_non_existing_image ok
    test_delete ok
    test_update ok
LimiterTest
    test_minute ok
    test_one_per_period ok
    test_second ok
    test_users_get_separate_buckets ok
    test_we_can_go_indefinitely_if_we_spread_out_requests ok
WSGIAppProxyTest
    test_200 ok
    test_403 ok
    test_failure ok
WSGIAppTest
    test_escaping ok
    test_good_urls ok
    test_invalid_methods ok
    test_invalid_urls ok
    test_response_to_delays ok
ServersTest
    test_create_backup_schedules ok
    test_create_instance ok
    test_delete_backup_schedules ok
    test_delete_server_instance ok
    test_get_all_server_details ok
    tes...

Revision history for this message
Vish Ishaya (vishvananda) wrote :

you need to add yourself to the Authors file as well

Revision history for this message
Jordan Rinke (jordanrinke) wrote :

> you need to add yourself to the Authors file as well
Fixed

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :
Download full text (21.4 KiB)

The attempt to merge lp:~jordanrinke/nova/lp707171 into lp:nova failed. Below is the output from the failed tests.

AdminAPITest
    test_admin_disabled ok
    test_admin_enabled ok
APITest
    test_exceptions_are_converted_to_faults ok
Test
    test_authorize_token ok
    test_authorize_user ok
    test_bad_token ok
    test_bad_user ok
    test_no_user ok
    test_token_expiry ok
TestLimiter
    test_authorize_token ok
TestFaults
    test_fault_parts ok
    test_raise ok
    test_retry_header ok
FlavorsTest
    test_get_flavor_by_id ok
    test_get_flavor_list ok
GlanceImageServiceTest
    test_create ok
    test_create_and_show_non_existing_image ok
    test_delete ok
    test_update ok
ImageControllerWithGlanceServiceTest
    test_get_image_details ok
    test_get_image_index ok
LocalImageServiceTest
    test_create ok
    test_create_and_show_non_existing_image ok
    test_delete ok
    test_update ok
LimiterTest
    test_minute ok
    test_one_per_period ok
    test_second ok
    test_users_get_separate_buckets ok
    test_we_can_go_indefinitely_if_we_spread_out_requests ok
WSGIAppProxyTest
    test_200 ok
    test_403 ok
    test_failure ok
WSGIAppTest
    test_escaping ok
    test_good_urls ok
    test_invalid_methods ok
    test_invalid_urls ok
    test_response_to_delays ok
ServersTest
    test_create_backup_schedules ok
    test_create_instance ok
    test_delete_backup_schedules ok
    test_delete_server_instance ok
    test_get_all_server_details ok
    tes...

Revision history for this message
Jordan Rinke (jordanrinke) wrote :

fixed again, hopefully the last one.

Revision history for this message
Jay Pipes (jaypipes) wrote :

lgtm.

review: Approve
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :
Download full text (20.8 KiB)

The attempt to merge lp:~jordanrinke/nova/lp707171 into lp:nova failed. Below is the output from the failed tests.

AdminAPITest
    test_admin_disabled ok
    test_admin_enabled ok
APITest
    test_exceptions_are_converted_to_faults ok
Test
    test_authorize_token ok
    test_authorize_user ok
    test_bad_token ok
    test_bad_user ok
    test_no_user ok
    test_token_expiry ok
TestLimiter
    test_authorize_token ok
TestFaults
    test_fault_parts ok
    test_raise ok
    test_retry_header ok
FlavorsTest
    test_get_flavor_by_id ok
    test_get_flavor_list ok
GlanceImageServiceTest
    test_create ok
    test_create_and_show_non_existing_image ok
    test_delete ok
    test_update ok
ImageControllerWithGlanceServiceTest
    test_get_image_details ok
    test_get_image_index ok
LocalImageServiceTest
    test_create ok
    test_create_and_show_non_existing_image ok
    test_delete ok
    test_update ok
LimiterTest
    test_minute ok
    test_one_per_period ok
    test_second ok
    test_users_get_separate_buckets ok
    test_we_can_go_indefinitely_if_we_spread_out_requests ok
WSGIAppProxyTest
    test_200 ok
    test_403 ok
    test_failure ok
WSGIAppTest
    test_escaping ok
    test_good_urls ok
    test_invalid_methods ok
    test_invalid_urls ok
    test_response_to_delays ok
ServersTest
    test_create_backup_schedules ok
    test_create_instance ok
    test_delete_backup_schedules ok
    test_delete_server_instance ok
    test_get_all_server_details ok
    tes...

Revision history for this message
Jordan Rinke (jordanrinke) wrote :

Fixed... again. (Has a 1 line change ever taken so many revisions before?)

lp:~jordanrinke/nova/lp707171 updated
612. By Jordan Rinke

Fixed spacing... AGAIN

Revision history for this message
Jay Pipes (jaypipes) wrote :

hehe

On Thu, Jan 27, 2011 at 11:45 AM, Jordan Rinke <email address hidden> wrote:
> Fixed... again. (Has a 1 line change ever taken so many revisions before?)
> --
> https://code.launchpad.net/~jordanrinke/nova/lp707171/+merge/47325
> You are reviewing the proposed merge of lp:~jordanrinke/nova/lp707171 into lp:nova.
>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Authors'
2--- Authors 2011-01-26 23:34:56 +0000
3+++ Authors 2011-01-27 17:45:45 +0000
4@@ -24,6 +24,7 @@
5 Joel Moore <joelbm24@gmail.com>
6 John Dewey <john@dewey.ws>
7 Jonathan Bryce <jbryce@jbryce.com>
8+Jordan Rinke <jordan@openstack.org>
9 Josh Durgin <joshd@hq.newdream.net>
10 Josh Kearney <josh.kearney@rackspace.com>
11 Joshua McKenty <jmckenty@gmail.com>
12
13=== modified file 'nova/virt/hyperv.py'
14--- nova/virt/hyperv.py 2011-01-25 23:20:55 +0000
15+++ nova/virt/hyperv.py 2011-01-27 17:45:45 +0000
16@@ -191,7 +191,7 @@
17 vcpus = long(instance['vcpus'])
18 procsetting.VirtualQuantity = vcpus
19 procsetting.Reservation = vcpus
20- procsetting.Limit = vcpus
21+ procsetting.Limit = 100000 # static assignment to 100%
22
23 (job, ret_val) = vs_man_svc.ModifyVirtualSystemResources(
24 vm.path_(), [procsetting.GetText_(1)])