CP-311123 support xenguest compression for live migration#6921
Conversation
| | "stream" -> | ||
| Stream | ||
| | str -> | ||
| warn "%s: unknown migration compressor %S" __FUNCTION__ str ; |
There was a problem hiding this comment.
| warn "%s: unknown migration compressor %S" __FUNCTION__ str ; | |
| warn "%s: unknown migration compressor %S" __FUNCTION__ !migration_compressor; |
For a more accurate message
There was a problem hiding this comment.
Now you would have an unused binding of str; why is this better or more readable? For me we are reporting the actual match.
There was a problem hiding this comment.
True, should make str into _. It's better IMO because we're reporting back what the user actually wrote - converting to lowercase is an internal processing step that doesn't need to be exposed to the user.
| @@ -1,5 +1,5 @@ | |||
| (* | |||
| * Copyright (C) Citrix Systems Inc. | |||
| a Copyright (C) Citrix Systems Inc. | |||
There was a problem hiding this comment.
this seems like a typo?
|
For reference, this is the corresponding emu-manager PR: xenserver/emu-manager#2. It's not yet merged. |
|
Tested this - so I'm confident it works as before by default and passes |
|
I think this is save to merge given that we won't pass new options to emu-manager/xenguest in the default configuration of xenopsd. The required change in xenopsd.conf (or the source code) acts like a feature flag. |
|
if the default xenopsd behaviour is unchanged, and a manual effort is needed to change the feature flag in xenopsd.conf to modify the default behaviour, then this change looks good to me |
psafont
left a comment
psafont
left a comment
There was a problem hiding this comment.
While I'd prefer lib/xenops_xenserver.ml to not have names that are implementation details of the xc backend, like Xenguest, the logic is good.
|
@psafont I am not fussed about how to name this method; realistically, xenguest is going to implement this first but overall, we can pass down to emu-manager anything that is different from |
* doc/content/design/migration-compression.md * xapi-project#6909 Add support for migration stream compression implemented in XenGuest. This complements the existing compression scheme that uses zstd to compress the VM and vGPU memory stream during compression. Which compression is used is controlled by xenopsd.conf and not through the API: we expect to switch to the new method in the long run. This patch modifies the existing code that sets up the compression/decompression pipeline to be used only when the "stream" method is used. When the "xenguest" method is used, we only need to pass a flag from emu-manager. The receiving side recognises the compression as part of the migration stream it receives and does not rely on outside signalling. For now we pass a generic -compress true" command line to xenguest but we could pass a different argument in the future. This patch relies on support by emu-manager to receive a new command line option "-compress" that it passes to xenguest. However, as long as Pool.migration-compressor = "stream" (the default), this parameter would not passed be to emu-manager. Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
lindig
force-pushed
the
private/christianlin/xenguest-compress-v2
branch
from
36b59ab to
ca5fac6
Compare
5 participants
Add support for migration stream compression implemented in XenGuest. This complements the existing compression scheme that uses zstd to compress the VM and vGPU memory stream during compression.
Which compression is used is controlled by xenopsd.conf and not through the API: we expect to switch to the new method in the long run.
This patch modifies the existing code that sets up the compression/decompression pipeline to be used only when the "stream" method is used. When the "xenguest" method is used, we only need to pass a flag from emu-manager. The receiving side recognises the compression as part of the migration stream it receives and does not rely on outside signalling.
For now we pass a generic -compress true" command line to xenguest but we could pass a different argument in the future.
This patch relies on support by emu-manager to receive a new command line option "-compress" that it passes to xenguest. However, as long as Pool.migration-compressor = "stream" (the default), this parameter would not passed be to emu-manager.