Re: [Qemu-devel] [PATCH] PPC: Fix via-cuda memory registration

[Alexander Graf]

On 12.09.2011, at 11:07, Avi Kivity wrote:

> On 09/11/2011 02:38 PM, Alexander Graf wrote:
>> Am 11.09.2011 um 12:41 schrieb Avi Kivity<address@hidden>:
>> 
>> >  On 09/08/2011 07:54 PM, Alexander Graf wrote:
>> >>  PS: Please test your patches. This one could have been found with an 
>> >> invocation
>> >>      as simple as "qemu-system-ppc". We boot into the OpenBIOS prompt by 
>> >> default,
>> >>      so you wouldn't even have required a guest image or kernel.
>> >>
>> >
>> >
>> >  Sorry about that.
>> >
>> >  Note that it's pretty hard to test these patches.  I often don't even 
>> > know which binary as the device->target relationship is not immediately 
>> > visible,
>> 
>> The patch was explicitly to convert ppc ;).
> 
> Yes, in this case.  Not in the general case.
> 
>> >  and I don't really know what to expect from the guest.
>> 
>> The very easy check-fundamentals thing to do for ppc is to execute 
>> qemu-system-ppc without arguments. It should drop you into an OF prompt. 
>> Both memory api bugs on ppc I've seen now would have been exposed with that.
>> 
>> I agree that we should have something slightly more sophisticated, but doing 
>> such a bare minimum test is almost for free to the tester and covers at 
>> least basic functionality :). I don't mind people introducibg subtle bugs in 
>> corner cases - these things happen. But an abort() when you execute the 
>> binary? That really shouldn't happen ever. This one is almost as bad.
> 
> Yeah.
> 
>> >  It would be best if we had a kvm-autotest testset for tcg, it would 
>> > probably run in just a few minutes and increase confidence in these 
>> > patches.
>> 
>> Yeah, I am using kvm-autotest today for regression testing, but it's very 
>> hard to tell it to run multiple different binaries. The target program 
>> variable can only be set for an execution job, making it impossible to run 
>> multiple targets in one autotest run.
> 
> Probably best to tell autotest about the directory, and let it pick up the 
> binary.  Still need some configuration to choose between qemu-kvm and 
> qemu-system-x86_64.
> 
> Lucas?
> 
>> 
>> Also, not all targets implement enough functionality for autotest. The e500 
>> machine for example doesn't support power off - real hw doesn't either. So 
>> we always have to kill the vm exposing potential data loss.
> 
> 'quit' from the monitor should cause any data loss.  You can get the guest to 
> sync by telling it via ssh (or just ignore the guest - who cares?)

At least currently we have a qcow2 check in place that fails with this method. 
That could just be a bug however.

> 
>> But that's probably gone by now with cache=unsafe fixed with your previous 
>> patches :). However that means that a simple test run takes quite a while 
>> already thanks to timeouts.
>> 
> 
> Why should you have any timeouts?  Sample the screen until you reach the 
> desired state, or perhaps ssh into the guest and test things, then (qemu) 
> quit.

As an alternative to shutting down the VM? Yes. As a replacement? No, because 
then we're never testing shutdown on machines that actually do support soft 
power off.


Alex