ioctl: Live Migration #911
Conversation
Continues efforts from the newly minted TP4159 PCIe Infrastructure for Live Migration specification by adding additional types and ioctl methods. Signed-off-by: Nate Thornton <[email protected]>
NateThornton
force-pushed
the
lm
branch
4 times, most recently
from
aaa49bc
to
fb711c9
Compare
| nvme_lm_migration_send; | ||
| nvme_lm_migration_recv; | ||
| nvme_lm_set_features_ctrl_data_queue; | ||
| nvme_lm_get_features_ctrl_data_queue; |
There was a problem hiding this comment.
Start a new section LIBNVME_1_12 and move them there.
| __u8 qt; | ||
| __u16 cntlid; | ||
| __u16 cdqid; | ||
| }; |
There was a problem hiding this comment.
Avoid any holes in the struct by rearranging the members in order to try to avoid ABI breakage. The only reliable alignment is to sort the members according their natural size: 64bit, pointers, 32bit, 16bit, 8bit. This is necessary that the version trick is working. Not nice, but it is what it is. We will address this in v2 :)
| .cdw10 = cdw10, | ||
| .addr = (__u64)(uintptr_t)args->data, | ||
| .data_len = args->data_len, | ||
| .timeout_ms = args->timeout, |
There was a problem hiding this comment.
Please avoid unrelated whitespace changes.
| }; | ||
|
|
||
| return nvme_get_features(&args); | ||
| } |
There was a problem hiding this comment.
The rest looks okay. Just move the ioctl.c and ioctl.h changes into a separate patch.
There was a problem hiding this comment.
Types moved to #913
| @@ -4429,6 +4430,7 @@ enum nvme_fid_supported_effects { | |||
| NVME_FID_SUPPORTED_EFFECTS_SCOPE_ENDGRP = 1 << 3, | |||
| NVME_FID_SUPPORTED_EFFECTS_SCOPE_DOMAIN = 1 << 4, | |||
| NVME_FID_SUPPORTED_EFFECTS_SCOPE_NSS = 1 << 5, | |||
| NVME_FID_SUPPORTED_EFFECTS_SCOPE_CDQSCP = 1 << 6, | |||
There was a problem hiding this comment.
The bit is just called Controller Data Queue (CDQSCP), no scope.
| @@ -9007,4 +9025,324 @@ struct nvme_ns_mgmt_host_sw_specified { | |||
| }; | |||
| #endif /* SWIG */ | |||
|
|
|||
| /** | |||
| * enum nvme_lm_select - Select (SEL): This field specifies the type of management operation to | |||
| * perform. | |||
There was a problem hiding this comment.
Does this render the documentation correct? Is the perform on the short description? I recenetly learned you can add a new line in the short description, but I think then you need to have a empty line between the header and the first paragraph.
There was a problem hiding this comment.
Is does in the man page. Is there somewhere else I should look to verify?
There was a problem hiding this comment.
Check the output of scripts/kernel-doc src/nvme/types.h
There was a problem hiding this comment.
Looks OK to me
| NVME_LM_RESUME = 1, | ||
| NVME_LM_SET_CONTROLLER_STATE = 2, | ||
| /* Migration Receive */ | ||
| NVME_LM_GET_CONTROLLER_STATE = 0, |
There was a problem hiding this comment.
I fiend this very surprising that the select field from different commands are mixed into one enum. Do we have this pattern already somewhere else?
Could this a problem when those commands get extended? And from past experience the chances for this are not zero.
There was a problem hiding this comment.
All the LM commands share the same cdw10 structure. I thought it better organized in one select block.
Would you rather I have an enum per command, where all fields pertaining to that command are located in one block? Or an enum per command per field?
There was a problem hiding this comment.
I broke these into per-command fields
There was a problem hiding this comment.
While I understand the motivation to interpret the spec and make it a bit more usable I am not sure if this is a good thing in the long run. We didn't do this for the rest of the spec. Maybe @keithbusch has some input on this.
| NVME_LM_MIGRATION_RECV_CTRL_SUSPENDED = 1, | ||
| NVME_LM_MIGRATION_RECV_CTRL_SUSPENDED_SHIFT = 0, | ||
| NVME_LM_MIGRATION_RECV_CTRL_SUSPENDED_MASK = 0x1, | ||
| }; |
There was a problem hiding this comment.
same comment as above, this seems to mix the definition from different commands into one enum.
Continues efforts from the newly minted TP4159 PCIe Infrastructure for Live Migration specification by adding additional types and ioctl methods.
Depends on completion of #913