On 05/04/2016 10:49 AM, Markus Armbruster wrote: > Type QJSON lets you build JSON text. Its interface mirrors (a subset > of) abstract JSON syntax. > > QAPI output visitors also produce JSON text. They assert their > preconditions and invariants, and therefore abort on incorrect use. > > Contrastingly, QJSON does *not* detect incorrect use. It happily > produces invalid JSON then. This is what migration wants. > > QJSON was designed for migration, and migration is its only user.
Worth calling out commits 0457d07..b174257 here? > Move it to migration/ for proper coverage by MAINTAINERS, and to deter > accidental use outside migration. > > +++ b/include/migration/vmstate.h > @@ -29,7 +29,7 @@ > #ifndef CONFIG_USER_ONLY > #include <migration/qemu-file.h> > #endif > -#include <qjson.h> > +#include "migration/qjson.h" I thought you weren't a fan of including .h from .h, where it was avoidable. But I guess you aren't adding any new .h, so much as converting an existing use. > + > #include "qemu/osdep.h" > -#include <qapi/qmp/qstring.h> > -#include <glib.h> > -#include <qjson.h> > -#include <qemu/module.h> > -#include <qom/object.h> > +#include "qapi/qmp/qstring.h" > +#include "migration/qjson.h" > +#include "qemu/module.h" > +#include "qom/object.h" Thanks for fixing the mis-use of <> while at it :) > +++ b/migration/vmstate.c > @@ -6,7 +6,6 @@ > #include "qemu/bitops.h" > #include "qemu/error-report.h" > #include "trace.h" > -#include "qjson.h" This is because you are relying on the .h doing it for you. As mentioned on the cover letter, Reviewed-by: Eric Blake <ebl...@redhat.com> whether or not you touch up the commit message to call out ids -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature