Re: [Qemu-devel] [PATCH V4 0/3] qemu-iotests: add test for fd passing via SCM rights

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 2013/9/11 15:32, Kevin Wolf 写道:
> Am 11.09.2013 um 04:38 hat Wenchao Xia geschrieben:
>> 于 2013/9/10 19:47, Kevin Wolf 写道:
>>> Am 09.09.2013 um 13:57 hat Stefan Hajnoczi geschrieben:
>>>> On Fri, Sep 06, 2013 at 11:24:31AM +0800, Wenchao Xia wrote:
>>>>> This series add test case for fd passing with unix socket at runtime. Since
>>>>> getfd and closefd interface will interact with monitor's data, so it will
>>>>> help to do regression test for monitor patches. Since python2 do not support
>>>>> sendmsg(), so a C helper program is added to do the job.
>>>>>
>>>>> v2:
>>>>>    1: add missing $ in the makefile rule.
>>>>>
>>>>> v3:
>>>>>    Address Eric's comments:
>>>>>    1: typo fix, remove "." in the end of error message, strick
>>>>> check argc as "!=", use EXIT_SUCCESS and EXIT_FAILURE as exit
>>>>> values, strict error check for strtol() call.
>>>>>    Address Luiz's comments:
>>>>>    1: change the helper program parameter as "bin<   socket-fd>   <   file-path>",
>>>>> the program open the file itself now, data parameter is removed and blank
>>>>> is always used as iov data, better usage tip message, folder the string parsing
>>>>> code into a function.
>>>>>    2: related change for helper program parameter change.
>>>>>    3: related change for helper program parameter change.
>>>>>    Other:
>>>>>    1: remove "LINK" rule in makefile, remove fd checking code inside send_fd()
>>>>> since it is already checked before calling, add '' around %s for path and
>>>>> number string in error message.
>>>>>    2: renamed fd_bin to bin in send_fd_scm() to tip better, add '' around %s
>>>>> for path in error message.
>>>>> v4:
>>>>>    Address Stefan's comments:
>>>>>    2: add space after # for comments, refined the comment's grammar.
>>>>>    3: add space after # for comments, refined the comment's grammar, add two
>>>>> test cases for error path.
>>>>>
>>>>> Wenchao Xia (3):
>>>>>    1 qemu-iotests: add unix socket help program
>>>>>    2 qemu-iotests: add infrastructure of fd passing via SCM
>>>>>    3 qemu-iotests: add tests for runtime fd passing via SCM rights
>>>>>
>>>>>   QMP/qmp.py                             |    6 ++
>>>>>   configure                              |    2 +-
>>>>>   tests/Makefile                         |    3 +-
>>>>>   tests/qemu-iotests/045                 |   51 ++++++++++++-
>>>>>   tests/qemu-iotests/045.out             |    4 +-
>>>>>   tests/qemu-iotests/check               |    1 +
>>>>>   tests/qemu-iotests/iotests.py          |   23 ++++++
>>>>>   tests/qemu-iotests/socket_scm_helper.c |  135 ++++++++++++++++++++++++++++++++
>>>>>   8 files changed, 220 insertions(+), 5 deletions(-)
>>>>>   create mode 100644 tests/qemu-iotests/socket_scm_helper.c
>>>> Reviewed-by: Stefan Hajnoczi<stefanha@redhat.com>
>>> I'm not completely convinced that it's a good idea to introduce a helper
>>> binary that isn't automatically rebuilt after an upgrade (I basically
>> Indeed it will not be rebuilt when code for qemu change, but will be
>> rebuilt when
>> socket_scm_helper.c change. The help program have no connection with
>> qemu's main code,
>> so I think it is safe.
>> The binaries built under tests/ will not be cleaned, so I think they
>> all have a chance to
>> stay as old version. If this is the issue you mean, I will send a
>> patch clean them, which
>> is what I planned to do sometimes ago . :)
> The difference is that the binaries under tests/ will usually only ever
> be called using 'make check', which does update them. In contrast, I
> usually run qemu-iotests by directly executing check -T -nocache -qcow2,
> which doesn't update anything.
>
> Kevin
>
   OK, I see the problem now. I guess "make" is done before to update qemu
binary, so the problem is how to trigger the build of helper. What about
add Makefile in test/qemu-iotests/ ? Then 'make check' in root dir, and
'check -T ..." both trigger the build.

>>> never run 'make check-block', which is the only way that would trigger
>>> it), but given this Reviewed-by, I've applied this to the block branch.
>>>
>>> Kevin
>>>