Skip to contentSkip to conversation

Preserve current monitor CPU when issuing HMP passthrough commands

PATCH In review Daniel P. Berrange 2 participants 2 messages Sep 1, 2011

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Preserve current monitor CPU when issuing HMP passthrough commands

From: "Daniel P. Berrange" <[email protected]>

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.

* monitor.c: Track 'mon_cpu' when doing HMP passthrough

---
 monitor.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
monitor.c +3 -0
index 421a65c..f99659d 100644
@@ -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));
Signature · 1 line
1.7.6
#1 lore raw
Quoted 21 lines · Daniel P. Berrange

From: "Daniel P. Berrange"<[email protected]>

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"}
Quoted 7 lines

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

Quoted 36 lines

* monitor.c: Track 'mon_cpu' when doing HMP passthrough

---
  monitor.c |    3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)
monitor.c +3 -0
index 421a65c..f99659d 100644
@@ -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));