| Message ID | 946357411.421760.1300463741632.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com |
|---|---|
| State | New |
| Headers | show |
On 2011-03-18 16:55, Ulrich Obergfell wrote: > > Part 3 of the patch implements the following options for the 'configure' script. > > --disable-hpet-driftfix > --enable-hpet-driftfix I see no benefit in this configurability. Just make the driftfix unconditionally available, runtime-disabled by default for now until it matured and there is no downside in enabling it all the time. Jan
>> Part 3 of the patch implements the following options for the >> 'configure' script. >> >> --disable-hpet-driftfix >> --enable-hpet-driftfix > > I see no benefit in this configurability. Just make the driftfix > unconditionally available, runtime-disabled by default for now until it > matured and there is no downside in enabling it all the time. Many Thanks Jan, I enclosed the code in '#ifdef CONFIG_HPET_DRIFTFIX ... #endif' so that it can be easily identified (and removed if the generic API would be implemented some day). Since the ifdef's are already there I added the configuration option for convenience. As you don't see any benefit in this option, I can remove that part of the patch. However, I'd suggest to keep the ifdef's and do the following: - Rename to '#ifdef HPET_DRIFTFIX ... #endif' to make it clear that this is not controlled via a configuration option. - Add '#define HPET_DRIFTFIX' to hw/hpet_emul.h. Do you agree ? Regards, Uli
On 2011-03-22 11:03, Ulrich Obergfell wrote: > >>> Part 3 of the patch implements the following options for the >>> 'configure' script. >>> >>> --disable-hpet-driftfix >>> --enable-hpet-driftfix >> >> I see no benefit in this configurability. Just make the driftfix >> unconditionally available, runtime-disabled by default for now until it >> matured and there is no downside in enabling it all the time. > > > Many Thanks Jan, > > I enclosed the code in '#ifdef CONFIG_HPET_DRIFTFIX ... #endif' > so that it can be easily identified (and removed if the generic API > would be implemented some day). Since the ifdef's are already there > I added the configuration option for convenience. As you don't see > any benefit in this option, I can remove that part of the patch. > However, I'd suggest to keep the ifdef's and do the following: > > - Rename to '#ifdef HPET_DRIFTFIX ... #endif' to make it clear that > this is not controlled via a configuration option. > > - Add '#define HPET_DRIFTFIX' to hw/hpet_emul.h. > > Do you agree ? Thanks to versioning control and feature-oriented commits, it's not very hard to identify what code changes relate to which feature additions. So I still don't see a need for that. Jan
diff -up ./configure.orig3 ./configure --- ./configure.orig3 2011-02-18 22:48:06.000000000 +0100 +++ ./configure 2011-03-13 12:43:44.870966181 +0100 @@ -140,6 +140,7 @@ linux_aio="" attr="" vhost_net="" xfs="" +hpet_driftfix="no" gprof="no" debug_tcg="no" @@ -749,6 +750,10 @@ for opt do ;; --enable-rbd) rbd="yes" ;; + --disable-hpet-driftfix) hpet_driftfix="no" + ;; + --enable-hpet-driftfix) hpet_driftfix="yes" + ;; *) echo "ERROR: unknown option $opt"; show_help="yes" ;; esac @@ -2602,6 +2607,7 @@ echo "Trace output file $trace_file-<pid echo "spice support $spice" echo "rbd support $rbd" echo "xfsctl support $xfs" +echo "HPET drift fix $hpet_driftfix" if test $sdl_too_old = "yes"; then echo "-> Your SDL version is too old - please upgrade to have SDL support" @@ -2893,6 +2899,10 @@ if test "$rbd" = "yes" ; then echo "CONFIG_RBD=y" >> $config_host_mak fi +if test "$hpet_driftfix" = "yes" ; then + echo "CONFIG_HPET_DRIFTFIX=y" >> $config_host_mak +fi + # USB host support case "$usb" in linux)
Part 3 of the patch implements the following options for the 'configure' script. --disable-hpet-driftfix --enable-hpet-driftfix Signed-off-by: Ulrich Obergfell <uobergfe@redhat.com>