[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 01/16] qemu: extract out qemuFetchConfigs from firmware



On 8/23/19 12:21 PM, Cole Robinson wrote:
> From: Marc-André Lureau <marcandre lureau redhat com>
> 
> The same config files disovery & priority rules are used for
> vhost-user backends.
> 
> Signed-off-by: Marc-André Lureau <marcandre lureau redhat com>
> Signed-off-by: Cole Robinson <crobinso redhat com>
> ---
>  src/qemu/Makefile.inc.am |   2 +
>  src/qemu/qemu_configs.c  | 183 +++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_configs.h  |  28 ++++++
>  src/qemu/qemu_firmware.c | 144 +-----------------------------
>  4 files changed, 215 insertions(+), 142 deletions(-)
>  create mode 100644 src/qemu/qemu_configs.c
>  create mode 100644 src/qemu/qemu_configs.h
> 
> diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
> index 30a9751cfd..f7a0fa4a84 100644
> --- a/src/qemu/Makefile.inc.am
> +++ b/src/qemu/Makefile.inc.am
> @@ -30,6 +30,8 @@ QEMU_DRIVER_SOURCES = \
>  	qemu/qemu_hotplugpriv.h \
>  	qemu/qemu_conf.c \
>  	qemu/qemu_conf.h \
> +	qemu/qemu_configs.c \
> +	qemu/qemu_configs.h \
>  	qemu/qemu_process.c \
>  	qemu/qemu_process.h \
>  	qemu/qemu_processpriv.h \

The code looks fine, but the 'configs' naming is too generic. I suggest
going verbose with it, qemu_interop_json.[ch]. Functions then should be
named qemuInteropJSONXXX

I think you could start the series with this, and qemu_vhost_user.c but
minus the one function that uses virDomainDef additions, and those two
bits could be applied independent of the rest of the series IMO

Thanks,
Cole


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]