Re: [Qemu-devel] [PATCH] Preserve current monitor CPU when issuing HMP passthrough commands

[Anthony Liguori]

On 09/01/2011 06:46 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"<address@hidden>
>
> Several info commands rely on the 'mon_cpu' field in the Monitor
> struct. This field can be updated using the 'cpu NN' command.
>
> The processing for HMP passthrough commands, however, does not
> use the global 'Monitor *' instance, instead creating a brand
> new instance on the stack for HMP command executed. This breaks
> anything setting/getting the current monitor CPU
>
>   $ ./x86_64-softmmu/qemu-system-x86_64 -cdrom ~/boot.iso -qmp stdio -smp 4
>   {"QMP": {"version": {"qemu": {"micro": 50, "minor": 15, "major": 0}, "package": ""}, 
> "capabilities": []}}
>   {"execute":"qmp_capabilities"}
>   {"return": {}}
>   {"execute":"human-monitor-command","arguments":{"command-line":"info cpus"}}
>   {"return": "* CPU #0: pc=0x000000000010017c (halted) thread_id=2570 \r\n  CPU #1: 
> pc=0x00000000000ff0a2 (halted) thread_id=2570 \r\n  CPU #2: pc=0x00000000000ff0a2 (halted) 
> thread_id=2570 \r\n  CPU #3: pc=0x00000000000ff0a2 (halted) thread_id=2570 \r\n"}
>   {"execute":"human-monitor-command","arguments":{"command-line":"cpu 2"}}
>   {"return": {}}
>   {"execute":"human-monitor-command","arguments":{"command-line":"info cpus"}}
>   {"return": "* CPU #0: pc=0x000000000010017c (halted) thread_id=2570 \r\n  CPU #1: 
> pc=0x00000000000ff0a2 (halted) thread_id=2570 \r\n  CPU #2: pc=0x00000000000ff0a2 (halted) 
> thread_id=2570 \r\n  CPU #3: pc=0x00000000000ff0a2 (halted) thread_id=2570 \r\n"}
>
> In that example, the '*' should have moved from CPU #0, to CPU #2
> but it did not.
>
> The simple fix is to just copy the existing 'mon_cpu' field into
> the new temporary Monitor instance, before the HMP command is
> run, and copy the updated value back to the global instance
> afterwards.

That's why human-monitor-command takes an additional argument for what 
CPU should be set.  This is intended behavior.  IOW, don't use the cpu 
command with the human monitor passthrough.

Regards,

Anthony Liguori

> * monitor.c: Track 'mon_cpu' when doing HMP passthrough
>
> Signed-off-by: Daniel P. Berrange<address@hidden>
> ---
>   monitor.c |    3 +++
>   1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 421a65c..f99659d 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -513,6 +513,7 @@ static int do_hmp_passthrough(Monitor *mon, const QDict 
> *params,
>       memset(&hmp, 0, sizeof(hmp));
>       qemu_chr_init_mem(&mchar);
>       hmp.chr =&mchar;
> +    hmp.mon_cpu = cur_mon->mon_cpu;
>
>       old_mon = cur_mon;
>       cur_mon =&hmp;
> @@ -521,6 +522,7 @@ static int do_hmp_passthrough(Monitor *mon, const QDict 
> *params,
>           ret = mon_set_cpu(qdict_get_int(params, "cpu-index"));
>           if (ret<  0) {
>               cur_mon = old_mon;
> +            cur_mon->mon_cpu = hmp.mon_cpu;
>               qerror_report(QERR_INVALID_PARAMETER_VALUE, "cpu-index", "a CPU 
> number");
>               goto out;
>           }
> @@ -528,6 +530,7 @@ static int do_hmp_passthrough(Monitor *mon, const QDict 
> *params,
>
>       handle_user_command(&hmp, qdict_get_str(params, "command-line"));
>       cur_mon = old_mon;
> +    cur_mon->mon_cpu = hmp.mon_cpu;
>
>       if (qemu_chr_mem_osize(hmp.chr)>  0) {
>           *ret_data = QOBJECT(qemu_chr_mem_to_qs(hmp.chr));