Re: [Qemu-devel] [PATCH 1/2] migration: Move qjson.[ch] to migration/

[Eric Blake]

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 <address@hidden>
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