On (Wed) 27 Jul 2011 [15:49:18], Markus Armbruster wrote:
> Amit Shah <amit.s...@redhat.com> writes:
>
> > Negative balloon values don't make sense, ignore them.
> >
> > Reported-by: Mike Cao <b...@redhat.com>
> > Signed-off-by: Amit Shah <amit.s...@redhat.com>
> > ---
> > I'm not sure if error_report is the right thing to use or should a new
> > qerror_report() be used. Luiz, comments?
>
> Since do_balloon() has been converted to qerror already, you should use
> qerror_report(). Something like this should do[*]:
>
> qerror_report(QERR_INVALID_PARAMETER_VALUE, "target", "a size")
>
> > balloon.c | 8 +++++++-
> > 1 files changed, 7 insertions(+), 1 deletions(-)
> >
> > diff --git a/balloon.c b/balloon.c
> > index cf9e3b2..e0ff97f 100644
> > --- a/balloon.c
> > +++ b/balloon.c
> > @@ -51,12 +51,16 @@ int qemu_add_balloon_handler(QEMUBalloonEvent
> > *event_func,
> > return 0;
> > }
> >
> > -static int qemu_balloon(ram_addr_t target)
> > +static int qemu_balloon(long long target)
> > {
> > if (!balloon_event_fn) {
> > return 0;
> > }
> > trace_balloon_event(balloon_opaque, target);
> > + if (target < 0) {
> > + error_report("Ignoring negative balloon value");
> > + return -1;
> > + }
> > balloon_event_fn(balloon_opaque, target);
> > return 1;
> > }
>
> Monitor argument type is 'M', i.e. target_long. Caller do_balloon() it
> as int64_t. Argument passing casts it to ram_addr_t, which is unsigned.
> Negative arguments get misinterpreted.
>
> You fix it by converting to long long instead, then rejecting negative
> arguments.
Ouch; that's crazy. I don't know why I thought qdict_get_int returned
long long.. I meant to use int64_t.
> I think do_balloon() is a more natural place to check the argument
> range. Permits keeping qemu_balloon()'s parameter type as is.
OK, done.
> > @@ -150,6 +154,8 @@ int do_balloon(Monitor *mon, const QDict *params,
> > if (ret == 0) {
> > qerror_report(QERR_DEVICE_NOT_ACTIVE, "balloon");
> > return -1;
> > + } else if (ret < 0) {
> > + return -1;
> > }
> >
> > cb(opaque, NULL);
>
> [*] Yes, that results in a sub-par error message for humans. Human
> users are advised to appreciate that the error message was created with
> proper object-oriented techniques.
Heh.
Amit
