-
Notifications
You must be signed in to change notification settings - Fork 281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Choose size of batch VM evacuation #5203
Choose size of batch VM evacuation #5203
Conversation
b432370
to
dac016c
Compare
b1dba54
to
7642f01
Compare
ocaml/idl/datamodel_host.ml
Outdated
; { | ||
param_type= Int | ||
; param_name= "evacuate_batch_size" | ||
; param_doc= "Number of VM to evacuate at the same time" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about The maximum number of VMs to be migrated per batch
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, it's more accurate. thx :)
c9375cd
to
eee67b9
Compare
ocaml/idl/datamodel_host.ml
Outdated
; param_name= "evacuate_batch_size" | ||
; param_doc= "The maximum number of VMs to be migrated per batch" | ||
; param_release= next_release | ||
; param_default= None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From https://github.com/xapi-project/xen-api/blob/master/ocaml/idl/ocaml_backend/gen_server.ml#L67, I understand is that None
means it is a mandatory parameter.
While this should not be as the old clients may send calls without the new parameter. Then Some <N>
would be a default value in this case. This could be tested with an old client.
eee67b9
to
ecdae7a
Compare
Okay so last forced push should take into account all comments, i'll let you close the conversations when satisfied. Meanwhile I tested giving a number and letting the default value take over, both works as expected. :) |
ecdae7a
to
d1a220a
Compare
New optional argument to `Host.evacuate`: `evacuate_batch_size` When provided uses it instead of `xapi.conf`'s `evacuation_batch_size` When not provided uses the `xapi.conf` option Fixes: xapi-project#5202 Signed-off-by: BenjiReis <benjamin.reis@vates.fr>
d1a220a
to
3ad1b3a
Compare
Fixes #7105 See xapi-project/xen-api#5202 See xapi-project/xen-api#5203 `host.evacuate`: try passing optional batch size argument. If not supported: remove it and try again.
New optional argument to
Host.evacuate
:evacuate_batch_size
When provided uses it instead ofxapi.conf
'sevacuation_batch_size
When not provided uses thexapi.conf
optionFixes: #5202