Re: [Qemu-devel] [PATCH 4/4] HMP: Introduce console command

[Lei Li <lilei@linux.vnet.ibm.com>]

On 10/29/2012 09:26 PM, Luiz Capitulino wrote:
> On Mon, 29 Oct 2012 12:18:03 +0800
> Lei Li <lilei@linux.vnet.ibm.com> wrote:
>
>> On 10/27/2012 01:43 AM, Luiz Capitulino wrote:
>>> On Fri, 26 Oct 2012 19:21:52 +0800
>>> Lei Li <lilei@linux.vnet.ibm.com> wrote:
>>>
>>>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
>>> I still don't understand how this command, in its current form, is
>>> different from memchar-write.
>>>
>>> One more comment below.
>> Hi Luiz,
>>
>> Yes, I have replied to it in patch series v4. You can look at it
>> as link below:
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2012-10/msg04568.html
> Unfortunately your answer doesn't answer my (honest) question. Can you actually
> show me how the console command is better than the memchar-write one? Maybe
> I'm missing something obvious...

As the example I post in the link, I think the main benefit for console command
is that it's more friendly.

A simple example, when we try to input "hello world" to this char device,
the memchar_write command would like this:

(qemu) memchar_write foo "hello world"

We need to add quotation marks for the string due to the monitor behaver.(Would
treat as another argument for the string which has space in it.)

The console command can avoid this like:

(qemu) console foo
foo: hello world
\r / ^]
(qemu)

It drops you into an interactive mode where it's like your at the serial console
and \r or ctrl-] escape sequences takes you back to the monitor suggested by Anthony.

I hope this can answer your question. :)

>>>> ---
>>>>    hmp-commands.hx |   25 +++++++++++++++++++++++++
>>>>    hmp.c           |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    hmp.h           |    1 +
>>>>    monitor.c       |   15 +++++++++++++++
>>>>    monitor.h       |    3 +++
>>>>    5 files changed, 96 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>>>> index df294eb..7cba42c 100644
>>>> --- a/hmp-commands.hx
>>>> +++ b/hmp-commands.hx
>>>> @@ -861,6 +861,31 @@ if the requested size is larger than it.
>>>>    ETEXI
>>>>    
>>>>        {
>>>> +        .name       = "console",
>>>> +        .args_type  = "chardev:s",
>>>> +        .params     = "chardev",
>>>> +        .help       = "Connect to the serial console from within the"
>>>> +                      "monitor, allow to write data to memchardev"
>>>> +                      "'chardev'. Exit from the console and return back"
>>>> +                      "to monitor by typing 'ctrl-]'",
>>>> +        .mhandler.cmd = hmp_console,
>>>> +    },
>>>> +
>>>> +STEXI
>>>> +@item console @var{device}
>>>> +@findex console
>>>> +
>>>> +Connect to the serial console from within the monitor, allow to write data
>>>> +to memchardev @var{chardev}. Exit from the console and return back to
>>>> +monitor by typing 'ctrl-]'.
>>>> +@example
>>>> +(qemu) console foo
>>>> +foo: data string...
>>>> +@end example
>>>> +
>>>> +ETEXI
>>>> +
>>>> +    {
>>>>            .name       = "migrate",
>>>>            .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
>>>>            .params     = "[-d] [-b] [-i] uri",
>>>> diff --git a/hmp.c b/hmp.c
>>>> index ef85736..d716410 100644
>>>> --- a/hmp.c
>>>> +++ b/hmp.c
>>>> @@ -1255,3 +1255,55 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
>>>>        qmp_screendump(filename, &err);
>>>>        hmp_handle_error(mon, &err);
>>>>    }
>>>> +
>>>> +enum escape_char
>>>> +{
>>>> +    ESCAPE_CHAR_CTRL_GS = 0x1d  /* ctrl-] used for escape */
>>>> +};
>>>> +
>>>> +static void hmp_read_console(Monitor *mon, const char *data,
>>>> +                             void *opaque)
>>>> +{
>>>> +    CharDriverState *chr = opaque;
>>>> +    uint32_t size = strlen(data);
>>>> +    enum DataFormat format = DATA_FORMAT_UTF8;
>>>> +    enum escape_char console_escape = ESCAPE_CHAR_CTRL_GS;
>>>> +
>>>> +    Error *err = NULL;
>>>> +
>>>> +    if (*data == console_escape) {
>>>> +        monitor_resume(mon);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    qmp_memchar_write(chr->label, size, data, 0, format, &err);
>>>> +
>>>> +    if (err) {
>>>> +        monitor_printf(mon, "%s\n", error_get_pretty(err));
>>>> +        error_free(err);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    monitor_read_command(mon, 1);
>>>> +}
>>>> +
>>>> +void hmp_console(Monitor *mon, const QDict *qdict)
>>>> +{
>>>> +    const char *device = qdict_get_str(qdict, "chardev");
>>>> +    CharDriverState *chr;
>>>> +    Error *err = NULL;
>>>> +
>>>> +    chr = qemu_chr_find(device);
>>>> +
>>>> +    if (!chr) {
>>>> +        error_set(&err, QERR_DEVICE_NOT_FOUND, device);
>>>> +        goto out;
>>>> +    }
>>> As I said before, I don't why chr is needed. It seems to me that passing
>>> 'device' down is enough.
>>>
>>>> +
>>>> +    if (monitor_read_console(mon, device, hmp_read_console, chr) < 0) {
>>>> +        monitor_printf(mon, "Connect to console %s failed\n", device);
>>>> +    }
>>>> +
>>>> +out:
>>>> +    hmp_handle_error(mon, &err);
>>>> +}
>>>> diff --git a/hmp.h b/hmp.h
>>>> index a5a0cfe..5b54a79 100644
>>>> --- a/hmp.h
>>>> +++ b/hmp.h
>>>> @@ -77,5 +77,6 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
>>>>    void hmp_closefd(Monitor *mon, const QDict *qdict);
>>>>    void hmp_send_key(Monitor *mon, const QDict *qdict);
>>>>    void hmp_screen_dump(Monitor *mon, const QDict *qdict);
>>>> +void hmp_console(Monitor *mon, const QDict *qdict);
>>>>    
>>>>    #endif
>>>> diff --git a/monitor.c b/monitor.c
>>>> index d17ae2d..7e90115 100644
>>>> --- a/monitor.c
>>>> +++ b/monitor.c
>>>> @@ -256,6 +256,21 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
>>>>        }
>>>>    }
>>>>    
>>>> +int monitor_read_console(Monitor *mon, const char *device,
>>>> +                         ReadLineFunc *readline_func, void *opaque)
>>>> +{
>>>> +    char prompt[60];
>>>> +
>>>> +    if (!mon->rs) {
>>>> +        return -1;
>>>> +    }
>>>> +
>>>> +    snprintf(prompt, sizeof(prompt), "%s: ", device);
>>>> +    readline_start(mon->rs, prompt, 0, readline_func, opaque);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>>    void monitor_flush(Monitor *mon)
>>>>    {
>>>>        if (mon && mon->outbuf_index != 0 && !mon->mux_out) {
>>>> diff --git a/monitor.h b/monitor.h
>>>> index b6e7d95..735bd1b 100644
>>>> --- a/monitor.h
>>>> +++ b/monitor.h
>>>> @@ -86,6 +86,9 @@ ReadLineState *monitor_get_rs(Monitor *mon);
>>>>    int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
>>>>                              void *opaque);
>>>>    
>>>> +int monitor_read_console(Monitor *mon, const char *device,
>>>> +                         ReadLineFunc *readline_func, void *opaque);
>>>> +
>>>>    int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret);
>>>>    
>>>>    int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret);
>>


-- 
Lei