Streamline command line interface #74
Conversation
|
Depends on xen-troops/zephyr#95 |
| return -EINVAL; | ||
| if (argc > 2) { | ||
| shell_print(shell, "%s %d\n", __func__, __LINE__); | ||
| domid = parse_domid(argc, argv); |
There was a problem hiding this comment.
is it really required to keep backward compatibility?
There was a problem hiding this comment.
I left the function to make the code more readable
There was a problem hiding this comment.
You changed all other cmd to be strict "xu <domain_id>", the diff for create cmd is that domain_id is optional. but why you left here "-d"? I think you can drop "-d", so "create" command will look the same as all other:
xu create [<domain_id>]
There was a problem hiding this comment.
Flags are commonly used for passing optional arguments in most terminal programs. They also help to remove confusion when there is more than one optional argument and I am almost sure that we will have more args in the future.
There was a problem hiding this comment.
ok. with possibility to have more options agree.
xen-shell-cmd/src/xen_cmds.c
Outdated
| LOG_ERR("Invalid domid passed to create cmd"); | ||
| return -EINVAL; | ||
| if (argc > 2) { | ||
| shell_print(shell, "%s %d\n", __func__, __LINE__); |
| return domain_unpause(domid); | ||
| } | ||
|
|
||
| SHELL_STATIC_SUBCMD_SET_CREATE( | ||
| subcmd_xu, | ||
| SHELL_CMD_ARG(create, NULL, | ||
| " Create Xen domain\n" | ||
| " Usage: create -d <domid>\n", | ||
| domu_create, 3, 0), | ||
| " Usage: create [-d <domid>]\n", |
There was a problem hiding this comment.
Why "-d" is kept here? it has been removed for all other commands.
There was a problem hiding this comment.
I believe, it is because this is an optional parameter
| @@ -632,7 +632,7 @@ int domain_create(struct xen_domain_cfg *domcfg, uint32_t domid) | |||
| memset(&config, 0, sizeof(config)); | |||
| prepare_domain_cfg(domcfg, &config); | |||
| config.grant_opts = XEN_DOMCTL_GRANT_version(1); | |||
| rc = xen_domctl_createdomain(domid, &config); | |||
| rc = xen_domctl_createdomain(&domid, &config); | |||
There was a problem hiding this comment.
Erm, should you make corresponding change to xen_domctl_createdomain definition? I can't it in this commit.
There was a problem hiding this comment.
It is in the PR to a zephyr repo linked above
| return domain_unpause(domid); | ||
| } | ||
|
|
||
| SHELL_STATIC_SUBCMD_SET_CREATE( | ||
| subcmd_xu, | ||
| SHELL_CMD_ARG(create, NULL, | ||
| " Create Xen domain\n" | ||
| " Usage: create -d <domid>\n", | ||
| domu_create, 3, 0), | ||
| " Usage: create [-d <domid>]\n", |
There was a problem hiding this comment.
I believe, it is because this is an optional parameter
@lorc Maybe we should create a separate issue or milestone regarding documentation and do it all at once? |
|
@Deedone @GrygiriiS |
My doc request here is not related to API, but to user level documentation (README), where I request to have two section related to this PR: 1 is how to enable domain ctrl shell interface and 2 is domain ctrl commands description. So as usual NACK without docs. |
If we pass domid 0 to the create_domain domctl, it will be replaced with the next available domain id by Xen. We need to get this id back to further manipulate the domain. Signed-off-by: Mykyta Poturai <[email protected]> Acked-by: Dmytro Firsov <[email protected]>
Bring command line interface of xu util to a more common form. Make mandatory arguments positional and allow optional arguments. Implement automatic domid selection for new domains if not provided. Simplify console command name. Signed-off-by: Mykyta Poturai <[email protected]> Acked-by: Dmytro Firsov <[email protected]>
|
@GrygiriiS, would #76 satisfy your requirement? |
Actually no, as SHELL CMD interface is not documented in readme, Hence, documentation is moved forward, thanks to @Deedone. I have no objection to this PR any more. |
Fixes #69
Bring command line interface of xu util to a more common form.
Make mandatory arguments positional and allow optional arguments.
Implement automatic domid selection for new domains if not provided.
Simplify console command name.