From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:59756) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QzD78-00080g-9c for qemu-devel@nongnu.org; Thu, 01 Sep 2011 15:39:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QzD76-0008QX-T7 for qemu-devel@nongnu.org; Thu, 01 Sep 2011 15:39:50 -0400 Received: from mail-gw0-f45.google.com ([74.125.83.45]:53010) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QzD76-0008QT-PL for qemu-devel@nongnu.org; Thu, 01 Sep 2011 15:39:48 -0400 Received: by gwb19 with SMTP id 19so1297569gwb.4 for ; Thu, 01 Sep 2011 12:39:48 -0700 (PDT) Message-ID: <4E5FDF7F.8080607@codemonkey.ws> Date: Thu, 01 Sep 2011 14:39:43 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1314877604-2720-1-git-send-email-berrange@redhat.com> In-Reply-To: <1314877604-2720-1-git-send-email-berrange@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] Preserve current monitor CPU when issuing HMP passthrough commands List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: qemu-devel@nongnu.org, Gleb Natapov On 09/01/2011 06:46 AM, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" > > 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 > --- > 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));