Message ID | 1263989255-13755-7-git-send-email-armbru@redhat.com |
---|---|
State | New |
Headers | show |
On 01/20/2010 06:07 AM, Markus Armbruster wrote: > Signed-off-by: Markus Armbruster<armbru@redhat.com> > --- > monitor.c | 4 ++-- > qemu-monitor.hx | 3 ++- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/monitor.c b/monitor.c > index 816f6fd..b9166c3 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -808,11 +808,11 @@ static void do_info_cpus(Monitor *mon, QObject **ret_data) > *ret_data = QOBJECT(cpu_list); > } > > -static void do_cpu_set(Monitor *mon, const QDict *qdict) > +static void do_cpu_set(Monitor *mon, const QDict *qdict, QObject **ret_data) > { > int index = qdict_get_int(qdict, "index"); > if (mon_set_cpu(index)< 0) > - monitor_printf(mon, "Invalid CPU index\n"); > + qemu_error_new(QERR_INVALID_CPU_INDEX); > Just out of curiousity, why introduce a new error verses using (QERR_INVALID_PARAMETER, "index")? Regards, Anthony Liguori
Anthony Liguori <anthony@codemonkey.ws> writes: > On 01/20/2010 06:07 AM, Markus Armbruster wrote: >> Signed-off-by: Markus Armbruster<armbru@redhat.com> >> --- >> monitor.c | 4 ++-- >> qemu-monitor.hx | 3 ++- >> 2 files changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/monitor.c b/monitor.c >> index 816f6fd..b9166c3 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -808,11 +808,11 @@ static void do_info_cpus(Monitor *mon, QObject **ret_data) >> *ret_data = QOBJECT(cpu_list); >> } >> >> -static void do_cpu_set(Monitor *mon, const QDict *qdict) >> +static void do_cpu_set(Monitor *mon, const QDict *qdict, QObject **ret_data) >> { >> int index = qdict_get_int(qdict, "index"); >> if (mon_set_cpu(index)< 0) >> - monitor_printf(mon, "Invalid CPU index\n"); >> + qemu_error_new(QERR_INVALID_CPU_INDEX); >> > > Just out of curiousity, why introduce a new error verses using > (QERR_INVALID_PARAMETER, "index")? To avoid changing the non-QMP monitor. If you don't care about that, I can revert 5/6 and update 6/6. Alternatively, here's a trick to reduce the number of client visible QMP errors while still keeping the old diagnostic on the non-QMP monitor: +#define QERR_INVALID_CPU_INDEX \ + "{ 'class': 'InvalidParameter, 'data': { 'name': 'index' } }" + #define QERR_INVALID_PARAMETER \ "{ 'class': 'InvalidParameter', 'data': { 'name': %s } }"
On 01/27/2010 02:00 AM, Markus Armbruster wrote: > Anthony Liguori<anthony@codemonkey.ws> writes: > > >> On 01/20/2010 06:07 AM, Markus Armbruster wrote: >> >>> Signed-off-by: Markus Armbruster<armbru@redhat.com> >>> --- >>> monitor.c | 4 ++-- >>> qemu-monitor.hx | 3 ++- >>> 2 files changed, 4 insertions(+), 3 deletions(-) >>> >>> diff --git a/monitor.c b/monitor.c >>> index 816f6fd..b9166c3 100644 >>> --- a/monitor.c >>> +++ b/monitor.c >>> @@ -808,11 +808,11 @@ static void do_info_cpus(Monitor *mon, QObject **ret_data) >>> *ret_data = QOBJECT(cpu_list); >>> } >>> >>> -static void do_cpu_set(Monitor *mon, const QDict *qdict) >>> +static void do_cpu_set(Monitor *mon, const QDict *qdict, QObject **ret_data) >>> { >>> int index = qdict_get_int(qdict, "index"); >>> if (mon_set_cpu(index)< 0) >>> - monitor_printf(mon, "Invalid CPU index\n"); >>> + qemu_error_new(QERR_INVALID_CPU_INDEX); >>> >>> >> Just out of curiousity, why introduce a new error verses using >> (QERR_INVALID_PARAMETER, "index")? >> > To avoid changing the non-QMP monitor. If you don't care about that, I > can revert 5/6 and update 6/6. > Yes, I don't mind changing error messages in the human monitor. > Alternatively, here's a trick to reduce the number of client visible QMP > errors while still keeping the old diagnostic on the non-QMP monitor: > > +#define QERR_INVALID_CPU_INDEX \ > + "{ 'class': 'InvalidParameter, 'data': { 'name': 'index' } }" > + > #define QERR_INVALID_PARAMETER \ > "{ 'class': 'InvalidParameter', 'data': { 'name': %s } }" > Not a bad idea if you think it's important. I don't think it is though. I think the chances of anyone relying on specific error messages on the monitor is very, very low. Regards, Anthony Liguori
diff --git a/monitor.c b/monitor.c index 816f6fd..b9166c3 100644 --- a/monitor.c +++ b/monitor.c @@ -808,11 +808,11 @@ static void do_info_cpus(Monitor *mon, QObject **ret_data) *ret_data = QOBJECT(cpu_list); } -static void do_cpu_set(Monitor *mon, const QDict *qdict) +static void do_cpu_set(Monitor *mon, const QDict *qdict, QObject **ret_data) { int index = qdict_get_int(qdict, "index"); if (mon_set_cpu(index) < 0) - monitor_printf(mon, "Invalid CPU index\n"); + qemu_error_new(QERR_INVALID_CPU_INDEX); } static void do_info_jit(Monitor *mon) diff --git a/qemu-monitor.hx b/qemu-monitor.hx index 1aa7818..415734a 100644 --- a/qemu-monitor.hx +++ b/qemu-monitor.hx @@ -573,7 +573,8 @@ ETEXI .args_type = "index:i", .params = "index", .help = "set the default CPU", - .mhandler.cmd = do_cpu_set, + .user_print = monitor_user_noop, + .mhandler.cmd_new = do_cpu_set, }, STEXI
Signed-off-by: Markus Armbruster <armbru@redhat.com> --- monitor.c | 4 ++-- qemu-monitor.hx | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-)