Re: [Qemu-devel] [PATCH 3/5] Add support for 'o' octet (bytes) format as monitor parameter.

[Luiz Capitulino <lcapitulino@redhat.com>]

On Tue, 28 Sep 2010 12:06:01 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Jes.Sorensen@redhat.com writes:
> 
> > From: Jes Sorensen <Jes.Sorensen@redhat.com>
> >
> > Octet format relies on strtobytes which supports K/k, M/m, G/g, T/t
> > suffixes and unit support for humans, like 1.3G
> >
> > Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> > ---
> >  monitor.c |   27 +++++++++++++++++++++++++++
> >  1 files changed, 27 insertions(+), 0 deletions(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index e602480..3630061 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -78,6 +78,11 @@
> >   * 'l'          target long (32 or 64 bit)
> >   * 'M'          just like 'l', except in user mode the value is
> >   *              multiplied by 2^20 (think Mebibyte)
> > + * 'o'          octets (aka bytes)
> > + *              user mode accepts an optional T, t, G, g, M, m, K, k
> > + *              suffix, which multiplies the value by 2^40 for
> > + *              suffixes T and t, 2^30 for suffixes G and g, 2^20 for
> > + *              M and m, 2^10 for K and k
> >   * 'f'          double
> >   *              user mode accepts an optional G, g, M, m, K, k suffix,
> >   *              which multiplies the value by 2^30 for suffixes G and
> > @@ -3594,6 +3599,28 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> >                  qdict_put(qdict, key, qint_from_int(val));
> >              }
> >              break;
> > +        case 'o':
> > +            {
> > +                int64_t val;
> > +                char *end;
> > +
> > +                while (qemu_isspace(*p))
> > +                    p++;
> > +                if (*typestr == '?') {
> > +                    typestr++;
> > +                    if (*p == '\0') {
> > +                        break;
> > +                    }
> > +                }
> > +                val = strtobytes(p, &end);
> > +                if (!val) {
> > +                    monitor_printf(mon, "invalid size\n");
> > +                    goto fail;
> > +                }
> > +                qdict_put(qdict, key, qint_from_int(val));
> > +                p = end;
> > +            }
> > +            break;
> >          case 'f':
> >          case 'T':
> >              {
> 
> This is incomplete, you need to update check_client_args_type() as
> well.  Testing with QMP should have made that obvious.

Right and we have the same issue with 'f' as you pointed out.

However, the real problem here is that I've duplicated this stuff
in QMP.

I see two solutions:

 1. Move all this stuff to common code (say monitor_args.c). This is
    easy and quick

 2. Move QMP away from args_type. This is probably the right thing to
    do, but then I'm afraid that we'll have two different kinds of
    'argument specifiers' (ie. QMP's and HMP's)