[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:38 PM, Cole Robinson wrote:
> 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

I realize now this file doesn't have anything json specific, so maybe
qemu_interop or qemu_interop_config ?

- Cole


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