| Message ID | 68962306385412f48130690d29a0349b322b9930.1276641524.git.jan.kiszka@web.de |
|---|---|
| State | New |
| Headers | show |
Jan Kiszka <jan.kiszka@web.de> writes: > From: Jan Kiszka <jan.kiszka@siemens.com> > > This enables command line completion inside option strings. A list of > expected key names and their completion type can be appended to the 'O' > inside parentheses ('O(key:type,...)'). The first use case is block > device completion for the 'drive' option of 'device_add'. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > monitor.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++--------- > qemu-monitor.hx | 2 +- > 2 files changed, 58 insertions(+), 12 deletions(-) > > diff --git a/monitor.c b/monitor.c > index c1006b4..3e0d862 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -68,6 +68,9 @@ > * 'O' option string of the form NAME=VALUE,... > * parsed according to QemuOptsList given by its name > * Example: 'device:O' uses qemu_device_opts. > + * Command completion for specific keys can be requested via > + * appending '(NAME:TYPE,...)' with 'F', 'B' as type. > + * Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path. > * Restriction: only lists with empty desc are supported > * TODO lift the restriction > * 'i' 32 bit integer Ugh. Replacement of args_type by a proper data structure is long overdue. We keep piling features into that poor, hapless string. Information on how to complete QemuOptsList options arguably belongs into the option description, i.e. QemuOptDesc. [...]
Markus Armbruster wrote: > Jan Kiszka <jan.kiszka@web.de> writes: > >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> This enables command line completion inside option strings. A list of >> expected key names and their completion type can be appended to the 'O' >> inside parentheses ('O(key:type,...)'). The first use case is block >> device completion for the 'drive' option of 'device_add'. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> monitor.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++--------- >> qemu-monitor.hx | 2 +- >> 2 files changed, 58 insertions(+), 12 deletions(-) >> >> diff --git a/monitor.c b/monitor.c >> index c1006b4..3e0d862 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -68,6 +68,9 @@ >> * 'O' option string of the form NAME=VALUE,... >> * parsed according to QemuOptsList given by its name >> * Example: 'device:O' uses qemu_device_opts. >> + * Command completion for specific keys can be requested via >> + * appending '(NAME:TYPE,...)' with 'F', 'B' as type. >> + * Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path. >> * Restriction: only lists with empty desc are supported >> * TODO lift the restriction >> * 'i' 32 bit integer > > Ugh. > > Replacement of args_type by a proper data structure is long overdue. We > keep piling features into that poor, hapless string. > > Information on how to complete QemuOptsList options arguably belongs > into the option description, i.e. QemuOptDesc. For sure, that would be better. I just wonder how much of it should be stuffed into this series. I guess I will drop this part for now, just focusing on what device_show makes direct use of. Same for separate args for HMP and QMP. Jan
Jan Kiszka <jan.kiszka@siemens.com> writes: > Markus Armbruster wrote: >> Jan Kiszka <jan.kiszka@web.de> writes: >> >>> From: Jan Kiszka <jan.kiszka@siemens.com> >>> >>> This enables command line completion inside option strings. A list of >>> expected key names and their completion type can be appended to the 'O' >>> inside parentheses ('O(key:type,...)'). The first use case is block >>> device completion for the 'drive' option of 'device_add'. >>> >>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >>> --- >>> monitor.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++--------- >>> qemu-monitor.hx | 2 +- >>> 2 files changed, 58 insertions(+), 12 deletions(-) >>> >>> diff --git a/monitor.c b/monitor.c >>> index c1006b4..3e0d862 100644 >>> --- a/monitor.c >>> +++ b/monitor.c >>> @@ -68,6 +68,9 @@ >>> * 'O' option string of the form NAME=VALUE,... >>> * parsed according to QemuOptsList given by its name >>> * Example: 'device:O' uses qemu_device_opts. >>> + * Command completion for specific keys can be requested via >>> + * appending '(NAME:TYPE,...)' with 'F', 'B' as type. >>> + * Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path. >>> * Restriction: only lists with empty desc are supported >>> * TODO lift the restriction >>> * 'i' 32 bit integer >> >> Ugh. >> >> Replacement of args_type by a proper data structure is long overdue. We >> keep piling features into that poor, hapless string. >> >> Information on how to complete QemuOptsList options arguably belongs >> into the option description, i.e. QemuOptDesc. > > For sure, that would be better. I just wonder how much of it should be > stuffed into this series. I guess I will drop this part for now, just > focusing on what device_show makes direct use of. Same for separate args > for HMP and QMP. Sensible. Speaking of args_type. We've accumulated too many ways to represent dynamic data types, and too many ways to describe data types. The most powerful dynamic data type is QObject. It comes with a useful plain-text representation (JSON). We lack a data type for describing QObjects. We use args_type to describe special QObjects, namely monitor command arguments. We've clearly stretched that to the limit and then some. We could use a general solution. JSON Schema as its plain-text representation could be nice. Then we have QemuOpts. It's more limited (flat list instead of trees), but it comes with a real data type to describe it (QemuOptsList). There's also QEMUOptionParameter. Serves a similar purpose. I don't know why we have both. Over in qdev-land, we find Property, which can be viewed a data type to describe (parts of) a struct. Some unification would be nice.
On Wed, 23 Jun 2010 12:28:27 +0200 Jan Kiszka <jan.kiszka@siemens.com> wrote: > Markus Armbruster wrote: > > Jan Kiszka <jan.kiszka@web.de> writes: > > > >> From: Jan Kiszka <jan.kiszka@siemens.com> > >> > >> This enables command line completion inside option strings. A list of > >> expected key names and their completion type can be appended to the 'O' > >> inside parentheses ('O(key:type,...)'). The first use case is block > >> device completion for the 'drive' option of 'device_add'. > >> > >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > >> --- > >> monitor.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++--------- > >> qemu-monitor.hx | 2 +- > >> 2 files changed, 58 insertions(+), 12 deletions(-) > >> > >> diff --git a/monitor.c b/monitor.c > >> index c1006b4..3e0d862 100644 > >> --- a/monitor.c > >> +++ b/monitor.c > >> @@ -68,6 +68,9 @@ > >> * 'O' option string of the form NAME=VALUE,... > >> * parsed according to QemuOptsList given by its name > >> * Example: 'device:O' uses qemu_device_opts. > >> + * Command completion for specific keys can be requested via > >> + * appending '(NAME:TYPE,...)' with 'F', 'B' as type. > >> + * Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path. > >> * Restriction: only lists with empty desc are supported > >> * TODO lift the restriction > >> * 'i' 32 bit integer > > > > Ugh. > > > > Replacement of args_type by a proper data structure is long overdue. We > > keep piling features into that poor, hapless string. > > > > Information on how to complete QemuOptsList options arguably belongs > > into the option description, i.e. QemuOptDesc. > > For sure, that would be better. I just wonder how much of it should be > stuffed into this series. I guess I will drop this part for now, just > focusing on what device_show makes direct use of. Same for separate args > for HMP and QMP. IIRC, the separate args idea use case was to allow commands like device_del to have an ID argument and a path argument, right? If so, I think it doesn't matter anymore as we have agreed on having a device argument which would accept both, even under QMP, right Markus? By the way, if you send patches 09/23, 10/23, 15/23, (maybe) 16/23, 21/23 and 22/23 in a different series, I could try pushing them in my next pull request.
Luiz Capitulino wrote: > On Wed, 23 Jun 2010 12:28:27 +0200 > Jan Kiszka <jan.kiszka@siemens.com> wrote: > >> Markus Armbruster wrote: >>> Jan Kiszka <jan.kiszka@web.de> writes: >>> >>>> From: Jan Kiszka <jan.kiszka@siemens.com> >>>> >>>> This enables command line completion inside option strings. A list of >>>> expected key names and their completion type can be appended to the 'O' >>>> inside parentheses ('O(key:type,...)'). The first use case is block >>>> device completion for the 'drive' option of 'device_add'. >>>> >>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >>>> --- >>>> monitor.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++--------- >>>> qemu-monitor.hx | 2 +- >>>> 2 files changed, 58 insertions(+), 12 deletions(-) >>>> >>>> diff --git a/monitor.c b/monitor.c >>>> index c1006b4..3e0d862 100644 >>>> --- a/monitor.c >>>> +++ b/monitor.c >>>> @@ -68,6 +68,9 @@ >>>> * 'O' option string of the form NAME=VALUE,... >>>> * parsed according to QemuOptsList given by its name >>>> * Example: 'device:O' uses qemu_device_opts. >>>> + * Command completion for specific keys can be requested via >>>> + * appending '(NAME:TYPE,...)' with 'F', 'B' as type. >>>> + * Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path. >>>> * Restriction: only lists with empty desc are supported >>>> * TODO lift the restriction >>>> * 'i' 32 bit integer >>> Ugh. >>> >>> Replacement of args_type by a proper data structure is long overdue. We >>> keep piling features into that poor, hapless string. >>> >>> Information on how to complete QemuOptsList options arguably belongs >>> into the option description, i.e. QemuOptDesc. >> For sure, that would be better. I just wonder how much of it should be >> stuffed into this series. I guess I will drop this part for now, just >> focusing on what device_show makes direct use of. Same for separate args >> for HMP and QMP. > > IIRC, the separate args idea use case was to allow commands like device_del > to have an ID argument and a path argument, right? If so, I think it doesn't > matter anymore as we have agreed on having a device argument which would > accept both, even under QMP, right Markus? To my understanding: As a leading element in qdev path, at least to address a device, maybe also to abbreviate only the beginning of a full path (that's currently to major remaining open issue). > > By the way, if you send patches 09/23, 10/23, 15/23, (maybe) 16/23, 21/23 > and 22/23 in a different series, I could try pushing them in my next > pull request. Do they need rebasing? If not, feel free to pick them up as you like. My series requires a v5 round anyway once discussion on path construction finally came to an end. Jan
On Mon, 28 Jun 2010 16:40:58 +0200 Jan Kiszka <jan.kiszka@siemens.com> wrote: > Luiz Capitulino wrote: > > On Wed, 23 Jun 2010 12:28:27 +0200 > > Jan Kiszka <jan.kiszka@siemens.com> wrote: > > > >> Markus Armbruster wrote: > >>> Jan Kiszka <jan.kiszka@web.de> writes: > >>> > >>>> From: Jan Kiszka <jan.kiszka@siemens.com> > >>>> > >>>> This enables command line completion inside option strings. A list of > >>>> expected key names and their completion type can be appended to the 'O' > >>>> inside parentheses ('O(key:type,...)'). The first use case is block > >>>> device completion for the 'drive' option of 'device_add'. > >>>> > >>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > >>>> --- > >>>> monitor.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++--------- > >>>> qemu-monitor.hx | 2 +- > >>>> 2 files changed, 58 insertions(+), 12 deletions(-) > >>>> > >>>> diff --git a/monitor.c b/monitor.c > >>>> index c1006b4..3e0d862 100644 > >>>> --- a/monitor.c > >>>> +++ b/monitor.c > >>>> @@ -68,6 +68,9 @@ > >>>> * 'O' option string of the form NAME=VALUE,... > >>>> * parsed according to QemuOptsList given by its name > >>>> * Example: 'device:O' uses qemu_device_opts. > >>>> + * Command completion for specific keys can be requested via > >>>> + * appending '(NAME:TYPE,...)' with 'F', 'B' as type. > >>>> + * Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path. > >>>> * Restriction: only lists with empty desc are supported > >>>> * TODO lift the restriction > >>>> * 'i' 32 bit integer > >>> Ugh. > >>> > >>> Replacement of args_type by a proper data structure is long overdue. We > >>> keep piling features into that poor, hapless string. > >>> > >>> Information on how to complete QemuOptsList options arguably belongs > >>> into the option description, i.e. QemuOptDesc. > >> For sure, that would be better. I just wonder how much of it should be > >> stuffed into this series. I guess I will drop this part for now, just > >> focusing on what device_show makes direct use of. Same for separate args > >> for HMP and QMP. > > > > IIRC, the separate args idea use case was to allow commands like device_del > > to have an ID argument and a path argument, right? If so, I think it doesn't > > matter anymore as we have agreed on having a device argument which would > > accept both, even under QMP, right Markus? > > To my understanding: As a leading element in qdev path, at least to > address a device, maybe also to abbreviate only the beginning of a full > path (that's currently to major remaining open issue). I'm ok with it if it's unambiguous. > > By the way, if you send patches 09/23, 10/23, 15/23, (maybe) 16/23, 21/23 > > and 22/23 in a different series, I could try pushing them in my next > > pull request. > > Do they need rebasing? If not, feel free to pick them up as you like. My > series requires a v5 round anyway once discussion on path construction > finally came to an end. Done for them all, except 16/23 which mentions device_show in the changelog. I should send a pull request until this Wednesday.
diff --git a/monitor.c b/monitor.c index c1006b4..3e0d862 100644 --- a/monitor.c +++ b/monitor.c @@ -68,6 +68,9 @@ * 'O' option string of the form NAME=VALUE,... * parsed according to QemuOptsList given by its name * Example: 'device:O' uses qemu_device_opts. + * Command completion for specific keys can be requested via + * appending '(NAME:TYPE,...)' with 'F', 'B' as type. + * Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path. * Restriction: only lists with empty desc are supported * TODO lift the restriction * 'i' 32 bit integer @@ -3353,6 +3356,11 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, QemuOptsList *opts_list; QemuOpts *opts; + if (*typestr == '(') { + while (*typestr++ != ')') { + assert(*typestr != '\0'); + } + } opts_list = qemu_find_opts(key); if (!opts_list || opts_list->desc->name) { goto bad_type; @@ -3857,12 +3865,30 @@ static const char *next_arg_type(const char *typestr) return (p != NULL ? ++p : typestr); } +static bool process_completion_type(char type, const char *str) +{ + switch(type) { + case 'F': + /* file completion */ + readline_set_completion_index(cur_mon->rs, strlen(str)); + file_completion(str); + return true; + case 'B': + /* block device name completion */ + readline_set_completion_index(cur_mon->rs, strlen(str)); + bdrv_iterate(block_completion_it, (void *)str); + return true; + default: + return false; + } +} + static void monitor_find_completion(const char *cmdline) { const char *cmdname; char *args[MAX_ARGS]; int nb_args, i, len; - const char *ptype, *str; + const char *ptype, *str, *opt, *sep; const mon_cmd_t *cmd; const KeyDef *key; @@ -3915,16 +3941,31 @@ static void monitor_find_completion(const char *cmdline) if (*ptype == '-' && ptype[1] != '\0') { ptype = next_arg_type(ptype); } + if (process_completion_type(*ptype, str)) { + goto cleanup; + } switch(*ptype) { - case 'F': - /* file completion */ - readline_set_completion_index(cur_mon->rs, strlen(str)); - file_completion(str); - break; - case 'B': - /* block device name completion */ - readline_set_completion_index(cur_mon->rs, strlen(str)); - bdrv_iterate(block_completion_it, (void *)str); + case 'O': + sep = strrchr(str, ','); + opt = sep ? sep + 1 : str; + sep = strchr(opt, '='); + if (!sep) { + break; + } + len = sep - opt; + str = sep + 1; + ptype += 2; + while (*ptype != ')') { + if (strlen(ptype) > len+1 && ptype[len] == ':' && + strncmp(ptype, opt, len) == 0) { + process_completion_type(ptype[len+1], str); + } + while (*ptype++ != ',') { + if (*ptype == ')') { + break; + } + } + } break; case 's': /* XXX: more generic ? */ @@ -3934,7 +3975,7 @@ static void monitor_find_completion(const char *cmdline) cmd_completion(str, cmd->name); } } else if (!strcmp(cmd->name, "sendkey")) { - char *sep = strrchr(str, '-'); + sep = strrchr(str, '-'); if (sep) str = sep + 1; readline_set_completion_index(cur_mon->rs, strlen(str)); @@ -4114,6 +4155,11 @@ static int monitor_check_qmp_args(const mon_cmd_t *cmd, QDict *args) cmd_args.flag = *p++; cmd_args.optional = 1; } else if (cmd_args.type == 'O') { + if (*p == '(') { + while (*p++ != ')') { + assert(*p != '\0'); + } + } opts_list = qemu_find_opts(qstring_get_str(cmd_args.name)); assert(opts_list); } else if (*p == '?') { diff --git a/qemu-monitor.hx b/qemu-monitor.hx index 0ea0555..b5d0f6d 100644 --- a/qemu-monitor.hx +++ b/qemu-monitor.hx @@ -660,7 +660,7 @@ ETEXI { .name = "device_add", - .args_type = "device:O", + .args_type = "device:O(drive:B)", .params = "driver[,prop=value][,...]", .help = "add device, like -device on the command line", .user_print = monitor_user_noop,