Re: [libvirt] [PATCH v2 0/3] pass the path of compress prog directly

2016-09-22 Thread Chen Hanxiao 

At 2016-09-22 19:05:33, "John Ferlan"  wrote:
>
>>>
>>> BTW: Patch 2 fails syntax-check
>>>
>> 
>> I make syntax-check again and passed on a centos7 machine.
>> Maybe it's a syntax-check bug?
>> 
>
>Don't think so...
>
>
>cppi: src/qemu/qemu_driver.c: line 3269: not properly indented
>maint.mk: incorrect preprocessor indentation
>cfg.mk:680: recipe for target 'sc_preprocessor_indentation' failed
>make: *** [sc_preprocessor_indentation] Error 1
>make: *** Waiting for unfinished jobs

I didn't install cppi on that centos7 machine.
Sorry for the noise.

>
>Line 3269:
>
>
># define VIR_QEMU_COMPRESS_CHECK(val, type)
>   \
>do {
>   \
>if (cfg->val) {
>
>...
>
>If the "# define" is changed to "#define", then the syntax-check passes.
>The usage of "# define" is done/allowed with an "#if" or "#ifdef" type
>construct in order to 'mark' that we're within some sort of preprocessor
>condition.
>
>Additionally those lines are too long - use 80 character columns

I need to re-read HACKING :)
Thanks for your help.

Regards,
- Chen



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 0/3] pass the path of compress prog directly

2016-09-22 Thread John Ferlan 

>>
>> BTW: Patch 2 fails syntax-check
>>
> 
> I make syntax-check again and passed on a centos7 machine.
> Maybe it's a syntax-check bug?
> 

Don't think so...


cppi: src/qemu/qemu_driver.c: line 3269: not properly indented
maint.mk: incorrect preprocessor indentation
cfg.mk:680: recipe for target 'sc_preprocessor_indentation' failed
make: *** [sc_preprocessor_indentation] Error 1
make: *** Waiting for unfinished jobs

Line 3269:


# define VIR_QEMU_COMPRESS_CHECK(val, type)
   \
do {
   \
if (cfg->val) {

...

If the "# define" is changed to "#define", then the syntax-check passes.
The usage of "# define" is done/allowed with an "#if" or "#ifdef" type
construct in order to 'mark' that we're within some sort of preprocessor
condition.

Additionally those lines are too long - use 80 character columns


John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 0/3] pass the path of compress prog directly

2016-09-21 Thread Chen Hanxiao 

At 2016-09-22 05:32:10, "John Ferlan"  wrote:
>
>
>On 09/19/2016 07:06 AM, Chen Hanxiao wrote:
>> From: Chen Hanxiao 
>> 
>> This series will pass the path of compress programe
>> directly to functions such as doCoreDump, qemuDomainSaveInternal, etc.
>> this can avoid one extra virFindFileInPath call in during virExec.
>> 
>> Also reduce code duplication by macros.
>> 
>> Chen Hanxiao (3):
>>   qemu_driver: pass path of compress prog directly
>>   qemu_driver: simplify compress prog check by macros
>>   qemu_driver: pass path of compress prog directly to doCoreDump
>> 
>>  src/qemu/qemu_driver.c | 242 
>> ++---
>>  1 file changed, 128 insertions(+), 114 deletions(-)
>> 
>
>
>Not really what I had in mind.
>
>I've posted a different approach instead. See
>
>http://www.redhat.com/archives/libvir-list/2016-September/msg00971.html

I'll review your series as soon as possible.

>
>BTW: Patch 2 fails syntax-check
>

I make syntax-check again and passed on a centos7 machine.
Maybe it's a syntax-check bug?

Regards,
- Chen


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 0/3] pass the path of compress prog directly

2016-09-21 Thread John Ferlan 


On 09/19/2016 07:06 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao 
> 
> This series will pass the path of compress programe
> directly to functions such as doCoreDump, qemuDomainSaveInternal, etc.
> this can avoid one extra virFindFileInPath call in during virExec.
> 
> Also reduce code duplication by macros.
> 
> Chen Hanxiao (3):
>   qemu_driver: pass path of compress prog directly
>   qemu_driver: simplify compress prog check by macros
>   qemu_driver: pass path of compress prog directly to doCoreDump
> 
>  src/qemu/qemu_driver.c | 242 
> ++---
>  1 file changed, 128 insertions(+), 114 deletions(-)
> 


Not really what I had in mind.

I've posted a different approach instead. See

http://www.redhat.com/archives/libvir-list/2016-September/msg00971.html

BTW: Patch 2 fails syntax-check


John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list