nvme: Live Migration #2560
nvme: Live Migration #2560
Conversation
|
From a quick look, it looks good. The compiler is not happy though: |
NateThornton
force-pushed
the
lm
branch
2 times, most recently
from
ea71a95 to
3aec9ef
Compare
| const char *sz = "CDQ Size (in dwords)"; | ||
| const char *cntlid = "Controller ID"; | ||
| const char *qt = "Queue Type (default: 0 = User Data Migration Queue)"; | ||
| const char *consent = "I consent this will not work and understand a CDQ cannot be mapped " |
|
@igaw Thanks for your feedback - I will make the corresponding changes to libnvme and then update this PR once available |
NateThornton
force-pushed
the
lm
branch
3 times, most recently
from
c2fd66f to
694e013
Compare
|
As we placed the new lm iocts in libnvme version 1.12, I think this change is still pending the next libnvme release. Then one follow-up change to meson.build is required to grab that new release. |
completions/_nvme
Outdated
| @@ -617,6 +618,118 @@ _nvme () { | |||
| ;; | |||
| esac | |||
| ;; | |||
| (lm) | |||
There was a problem hiding this comment.
@NateThornton Self-reminder these need updating too
There was a problem hiding this comment.
and while at, also split the tab completions changes into a separate patch. don't forget to update the docs too :)
subprojects/libnvme.wrap
Outdated
| @@ -1,6 +1,6 @@ | |||
| [wrap-git] | |||
| url = https://github.com/linux-nvme/libnvme.git | |||
| revision = 8efcec730603ffcc379ba0bf821e4c24ec87715b | |||
| revision = 27ed889d0b1917919763816c97a49bb6e29d4547 | |||
There was a problem hiding this comment.
btw, could split the libnvme build dependency into a separate patch and have it first? This makes any backporting way simpler.
There was a problem hiding this comment.
Usually you just add a patch to a PR, no need for an extra PR just to update the dependency.
nvme-print-json.c
Outdated
| obj_add_uint(r, "mcumq", le16_to_cpu(ctrl->mcudmq)); | ||
| obj_add_uint(r, "mcmr", le16_to_cpu(ctrl->mcmr)); | ||
| obj_add_uint(r, "nmcmr", le16_to_cpu(ctrl->nmcmr)); | ||
| obj_add_uint(r, "mcdqpc", le16_to_cpu(ctrl->mcdqpc)); |
There was a problem hiding this comment.
the nvme-print*.c updates should be a separate patch.
There was a problem hiding this comment.
Glad you pointed this out because it's already included in #2607. Surprised I didn't get a merge conflict.
plugins/lm/lm-nvme.c
Outdated
| return err; | ||
| } | ||
|
|
||
| static void json_controller_state_data(struct nvme_lm_controller_state_data *data, size_t len) |
There was a problem hiding this comment.
So it might make sense to support builds without json-c dependencies for this new plugin. Most plugins depend on json-c to be available. @ikegami-t started to split the plugins so they work also when json-c is missing (see the ocp plugin). I think it would be good to have this from the beginning as it is currently small code base. LM is something in the base spec, so not unlikely to change or get udpated soonish :)
There was a problem hiding this comment.
Refactored following ocp plugin as an example.
NateThornton
force-pushed
the
lm
branch
2 times, most recently
from
34a081d to
c9af9e5
Compare
|
The commit subject should be prefix, e.g. Reverted nvme-print-stdout.c changes: still something in the first patch. As I said, I don't mind that nvme-print-stdout.c and nvme-print-json.c get updated, it just deserves it's own patch. Also why patch 2 removing stuff added previously. These patches should really be consisting out of new code being added and a tiny change in the meson.build file. And the build is unhappy due to printf formatting issues. |
NateThornton
force-pushed
the
lm
branch
4 times, most recently
from
d187075 to
84f5e93
Compare
|
I should have clarified my intent to squash the commits after they were individually reviewed. Everything now is in one commit and the build is passing. I think the remaining warnings are acceptable, but if something needs to be done to clean them up LMK. As we placed the new lm_* ioctls in release 12 of libnvme, does this merge need to wait until that is released? |
Implementation of TP 4159 PCIe Infrastructure for Live Migration plugin. Includes command support for Track Send, Migration Receive, Migration Send, and Controller Data Queue; Identify Controller LM related fields; Bash and ZSH completions. Changes are isolated to the User Data Migration subset, with Track Memory functionality deferred to a future commit. Signed-off-by: Nate Thornton <[email protected]>
|
I've dropped the changes in Thanks! |
Implementation of TP 4159 PCIe Infrastructure for Live Migration plugin. Includes command support for Track Send, Migration Receive, Migration Send, and Controller Data Queue; Identify Controller LM related fields; Bash and ZSH completions.
Changes are isolated to the User Data Migration subset, with Track Memory functionality deferred to a future commit.
TP4159 PCIe Infrastructure for Live Migration 2024.07.30 Ratified.pdf