Re: [Autotest] [PATCH] Add a client-side test qemu_iotests

Re: [Autotest] [PATCH] Add a client-side test qemu_iotests

[Martin Bligh]

From: root <root@dhcp-66-70-57.nay.redhat.com>


Signed-off-by: root <root@dhcp-66-70-57.nay.redhat.com>
---


;-)
Can we get these signed off by a person please? Preferably with a real email
address (see the DCO, in top level directory)

Re: [Autotest] [PATCH] Add a client-side test qemu_iotests

[Yolkfull Chow]

On 07/02/2009 11:49 AM, Martin Bligh wrote:
> From: root<root@dhcp-66-70-57.nay.redhat.com>
>
>
> Signed-off-by: root<root@dhcp-66-70-57.nay.redhat.com>
> ---
>
>
> ;-)
> Can we get these signed off by a person please? Preferably with a real email
> address (see the DCO, in top level directory)
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>    
Sure. Will re-send it now. :)

-- 
Yolkfull
Regards,

Re: [Autotest] [PATCH] Add a client-side test qemu_iotests

[Yolkfull Chow]

On 07/02/2009 12:24 PM, sudhir kumar wrote:
> Please send the tarball as a separate attachment. Your email is scary.
OK, I will resend the patch. Sorry for this scary email. :)


-- 
Yolkfull
Regards,

Re: [Autotest] [PATCH] Add a client-side test qemu_iotests

[Lucas Meneghel Rodrigues]

On Thu, 2009-07-02 at 12:46 +0800, Yolkfull Chow wrote:
> On 07/02/2009 12:24 PM, sudhir kumar wrote:
> > Please send the tarball as a separate attachment. Your email is scary.
> OK, I will resend the patch. Sorry for this scary email. :)

We usually send binary files as attachments instead of binary diffs just
for the sake of clarity. No need to apologize though :)

Lucas

Re: [Autotest] [PATCH] Add a client-side test qemu_iotests

[Lucas Meneghel Rodrigues]

On Thu, 2009-07-02 at 13:10 +0800, Yolkfull Chow wrote:
> Signed-off-by: Yolkfull Chow <yzhou@redhat.com>
> ---
>   client/tests/qemu_iotests/control         |   31 ++++++++++++++
>   client/tests/qemu_iotests/qemu_iotests.py |   63 
> +++++++++++++++++++++++++++++
>   2 files changed, 94 insertions(+), 0 deletions(-)
>   create mode 100644 client/tests/qemu_iotests/control
>   create mode 100755 client/tests/qemu_iotests/qemu_iotests.py

Ok, I've been trough the test and took a look at the testsuite itself.
Both look good. The testsuite requires a very current qemu-img package,
due to the use of qemu-io (F11 for instance doesn't ship it). So if you
want to try this make sure you use a current version of the kvm
utilities.

+1

Lucas

Re: [Autotest] [PATCH] Add a client-side test qemu_iotests

[Yolkfull Chow]

On 07/03/2009 08:23 AM, Lucas Meneghel Rodrigues wrote:
> On Thu, 2009-07-02 at 13:10 +0800, Yolkfull Chow wrote:
>    
>> Signed-off-by: Yolkfull Chow<yzhou@redhat.com>
>> ---
>>    client/tests/qemu_iotests/control         |   31 ++++++++++++++
>>    client/tests/qemu_iotests/qemu_iotests.py |   63
>> +++++++++++++++++++++++++++++
>>    2 files changed, 94 insertions(+), 0 deletions(-)
>>    create mode 100644 client/tests/qemu_iotests/control
>>    create mode 100755 client/tests/qemu_iotests/qemu_iotests.py
>>      
> Ok, I've been trough the test and took a look at the testsuite itself.
> Both look good. The testsuite requires a very current qemu-img package,
> due to the use of qemu-io (F11 for instance doesn't ship it). So if you
> want to try this make sure you use a current version of the kvm
> utilities.
>    
I had compiled upstream qemu codes and run this test based on it, worked 
fine though two cases failed for some reason.
> +1
>
> Lucas
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>    


-- 
Yolkfull
Regards,

Re: [Autotest] [PATCH] Add a client-side test qemu_iotests

[Lucas Meneghel Rodrigues]

On Thu, 2009-07-02 at 21:23 -0300, Lucas Meneghel Rodrigues wrote:
> Ok, I've been trough the test and took a look at the testsuite itself.
> Both look good. The testsuite requires a very current qemu-img package,
> due to the use of qemu-io (F11 for instance doesn't ship it). So if you
> want to try this make sure you use a current version of the kvm
> utilities.

It just occurred to me some suggestions based on my last comment:
 * Make sure we have the qemu, qemu-img and the qemu-io ELF files
available to execute the test suite, and if we doesn't, raise a
error.TestError() exception.
 * Make it possible to specify an alternate prefix to look for those
ELFs, and make sure the test suite can find them (ie, appending the path
to os.environ['PATH']).

Yolkfull, please take a look at implementing those suggestions.

Thanks again for your work!

Lucas

Re: [Autotest] [PATCH] Add a client-side test qemu_iotests

[Yolkfull Chow]

On 07/03/2009 01:03 PM, Lucas Meneghel Rodrigues wrote:
> On Thu, 2009-07-02 at 21:23 -0300, Lucas Meneghel Rodrigues wrote:
>    
>> Ok, I've been trough the test and took a look at the testsuite itself.
>> Both look good. The testsuite requires a very current qemu-img package,
>> due to the use of qemu-io (F11 for instance doesn't ship it). So if you
>> want to try this make sure you use a current version of the kvm
>> utilities.
>>      
> It just occurred to me some suggestions based on my last comment:
>   * Make sure we have the qemu, qemu-img and the qemu-io ELF files
> available to execute the test suite, and if we doesn't, raise a
> error.TestError() exception.
>   * Make it possible to specify an alternate prefix to look for those
> ELFs, and make sure the test suite can find them (ie, appending the path
> to os.environ['PATH']).
>
> Yolkfull, please take a look at implementing those suggestions.
>
> Thanks again for your work!
>
> Lucas
>
>    
Lucas, OK, I really forgot to set the PATH of 'qemu', 'qemu-img' and 
'qemu-io'. I will do modifications according to your wonderful suggestions.

Thank you so much! :-)

-- 
Yolkfull
Regards,

Re: [Autotest] [PATCH] Add a client-side test qemu_iotests

[Yolkfull Chow]

On 07/03/2009 01:03 PM, Lucas Meneghel Rodrigues wrote:
> On Thu, 2009-07-02 at 21:23 -0300, Lucas Meneghel Rodrigues wrote:
>    
>> Ok, I've been trough the test and took a look at the testsuite itself.
>> Both look good. The testsuite requires a very current qemu-img package,
>> due to the use of qemu-io (F11 for instance doesn't ship it). So if you
>> want to try this make sure you use a current version of the kvm
>> utilities.
>>      
> It just occurred to me some suggestions based on my last comment:
>   * Make sure we have the qemu, qemu-img and the qemu-io ELF files
> available to execute the test suite, and if we doesn't, raise a
> error.TestError() exception.
>    
I modified.
>   * Make it possible to specify an alternate prefix to look for those
> ELFs, and make sure the test suite can find them (ie, appending the path
> to os.environ['PATH']).
>    
Lucas, do we really need to find those ELFs within the whole disk if 
they are not in os.environ['PATH']?  That's not testsuite's purpose I 
think.

What's your opinion?
> Yolkfull, please take a look at implementing those suggestions.
>
> Thanks again for your work!
>
> Lucas
>
>    


-- 
Yolkfull
Regards,

Re: [Autotest] [PATCH] Add a client-side test qemu_iotests

[Lucas Meneghel Rodrigues]

On Fri, 2009-07-03 at 17:18 +0800, Yolkfull Chow wrote:
> Lucas, do we really need to find those ELFs within the whole disk if 
> they are not in os.environ['PATH']?  That's not testsuite's purpose I 
> think.
> 
> What's your opinion?

Sure it's not. My point was just to allow the user to provide an
additional path (say self.srcdir or something like that) that could be
added to the environmental variable PATH. This would be useful if we
build/install the ELFs under a non standard location on a previous test.
No need to walk through the entire filesystem to find the binaries.

Re: [Autotest] [PATCH] Add a client-side test qemu_iotests

[Lucas Meneghel Rodrigues]

On Mon, 2009-07-06 at 13:17 +0800, Yolkfull Chow wrote:

> Hi Lucas, since the qemu_iotests test suite itself has checked qemu,
> qemu-img and qemu-io ELF files before running, I would skip the
> duplicated step.

Fair enough. Making the check on the autotest module would make things
more apparent to the user (we would throw a TestFailure with the
reason), but I see your point.

> I added 'qemu_path' option for users to specify their alternative PATH
> containing these ELF files. 
> 
> > +        if qemu_path: 
> > +            os.environ['PATH'] += ":" + qemu_path 
> > + 

Good.

Martin, any comments? The test module and the qemu-iotests suite are
looking good, can we commit it?

Lucas