Re: [Qemu-devel] [RFC PATCH V3 2/6] icount: Add align option to icount

[Sebastian Tanase <sebastian.tanase@openwide.fr>]

----- Mail original -----
> De: "Paolo Bonzini" <pbonzini@redhat.com>
> À: "Sebastian Tanase" <sebastian.tanase@openwide.fr>, qemu-devel@nongnu.org
> Cc: aliguori@amazon.com, afaerber@suse.de, rth@twiddle.net, "peter maydell" <peter.maydell@linaro.org>,
> michael@walle.cc, alex@alex.org.uk, stefanha@redhat.com, lcapitulino@redhat.com, crobinso@redhat.com,
> armbru@redhat.com, wenchaoqemu@gmail.com, quintela@redhat.com, kwolf@redhat.com, mjt@tls.msk.ru, mst@redhat.com
> Envoyé: Lundi 30 Juin 2014 18:33:38
> Objet: Re: [RFC PATCH V3 2/6] icount: Add align option to icount
> 
> Il 30/06/2014 15:59, Sebastian Tanase ha scritto:
> > The align option is used for activating the align algorithm
> > in order to synchronise the host clock and the guest clock.
> >
> > Signed-off-by: Sebastian Tanase <sebastian.tanase@openwide.fr>
> > Tested-by: Camille Bégué <camille.begue@openwide.fr>
> > ---
> >  cpus.c                | 13 +++++++++++++
> >  include/qemu-common.h |  1 +
> >  qemu-options.hx       | 15 +++++++++++++--
> >  vl.c                  |  4 ++++
> >  4 files changed, 31 insertions(+), 2 deletions(-)
> >
> > diff --git a/cpus.c b/cpus.c
> > index dcca96a..3fa8ce7 100644
> > --- a/cpus.c
> > +++ b/cpus.c
> > @@ -448,13 +448,24 @@ void configure_icount(QemuOpts *opts, Error
> > **errp)
> >      vmstate_register(NULL, 0, &vmstate_timers, &timers_state);
> >      option = qemu_opt_get(opts, "shift");
> >      if (!option) {
> > +        if (qemu_opt_get(opts, "align") != NULL) {
> > +            error_setg(errp, "Please specify shift option when
> > using align");
> > +        }
> >          return;
> >      }
> > +    icount_align_option = qemu_opt_get_bool(opts, "align", false);
> >      /* When using -icount shift, the shift option will be
> >         misinterpreted as a boolean */
> >      if (strcmp(option, "on") == 0 || strcmp(option, "off") == 0) {
> >          error_setg(errp, "The shift option must be a number or
> >          auto");
> >      }
> > +    /* When using icount [shift=]N|auto -icount align, shift
> > becomes
> > +       align therefore we have to specify align=on|off.
> > +       We do however inform the user whenever the case. */
> > +    if (strcmp(option, "align") == 0) {
> > +        error_setg(errp, "Please specify align=on or off so as not
> > "
> > +         "to create confusion between the shift and align
> > options");
> > +    }
> 
> Can you prepare a follow-up (on top of this patch) that instead adds
> proper error handling to the strtol call below?  Then these strcmps
> can
> probably go away.
> 
> Paolo
> 
Sure, no problem.

Thanks for the advice.

Sebastian