On 10/06/2015 02:00 PM, Paolo Bonzini wrote: > 1) Compile files once > > 2) Move include file from replay/replay.h to include/sysemu/replay.h. > > 3) Fix Error usage > > 4) cleanup timerlistgroup_deadline_ns a bit and allow clock jump > notifiers to run > > 5) move replay-user.c to stubs/ > ---
> +++ b/include/qapi/qmp/qerror.h > @@ -107,6 +107,6 @@ > "this feature or command is not currently supported" > > #define QERR_REPLAY_NOT_SUPPORTED \ > - ERROR_CLASS_GENERIC_ERROR, "Record/replay feature is not supported for > '%s'" > + "Record/replay feature is not supported for '%s'" We should not be adding new #defines to this file. Instead, inline the message into the callers that do error_setg() (I see hw/bt/hci.c as the first such caller). > +++ b/qapi/common.json > @@ -22,15 +22,11 @@ > # @KVMMissingCap: the requested operation can't be fulfilled because a > # required KVM capability is missing > # > -# @ReplayNotSupported: the requested feature is not supported with > -# record/replay mode enabled > -# > # Since: 1.2 > ## > { 'enum': 'ErrorClass', > 'data': [ 'GenericError', 'CommandNotFound', 'DeviceEncrypted', > - 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap', > - 'ReplayNotSupported' ] } > + 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap' ] } Thank you for this. We definitely do not want to be adding new error classes without a very strong reason, and even if such classes are added, they must properly be documented as 'Since 2.5'. > +++ b/vl.c > @@ -122,7 +122,7 @@ int main(int argc, char **argv) > #include "qapi-event.h" > #include "exec/semihost.h" > #include "crypto/init.h" > -#include "replay/replay.h" > +#include "sysemu/replay.h" > #include "qapi/qmp/qerror.h" > > #define MAX_VIRTIO_CONSOLES 1 > @@ -851,7 +851,7 @@ static void configure_rtc(QemuOpts *opts) > } else if (!strcmp(value, "localtime")) { > Error *blocker = NULL; > rtc_utc = 0; > - error_set(&blocker, ERROR_CLASS_REPLAY_NOT_SUPPORTED, > + error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, > "-rtc base=localtime"); > replay_add_blocker(blocker); > } else { > @@ -1258,7 +1258,7 @@ static void smp_parse(QemuOpts *opts) > > if (smp_cpus > 1 || smp_cores > 1 || smp_threads > 1) { > Error *blocker = NULL; > - error_set(&blocker, ERROR_CLASS_REPLAY_NOT_SUPPORTED, "smp"); > + error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp"); > replay_add_blocker(blocker); Okay, I see that there is more than one location with the same failure, which is where using the #define sort of makes it nicer to guarantee a consistent message. But in general, use of error_setg() with a macro that passes a %s to printf at a distance is ugly, and should be avoided compared to just inlining the error message directly or writing a helper method that can properly set a consistent message. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature