Re: [Qemu-devel] [PATCH for 1.6] mips: revert commit b332d24a8e1290954029814d09156b06ede358e2

["Andreas Färber" <afaerber@suse.de>]

Am 05.08.2013 18:43, schrieb Anthony Liguori:
> Andreas Färber <afaerber@suse.de> writes:
> 
>> Am 05.08.2013 00:06, schrieb Aurelien Jarno:
>>> On Sun, Aug 04, 2013 at 02:03:20PM +0200, Andreas Färber wrote:
>>>> Am 04.08.2013 00:02, schrieb Aurelien Jarno:
>>>>> Now that this code path is not triggered anymore during the tests,
>>>>> revert commit b332d24a8e1290954029814d09156b06ede358e2. Booting a MIPS
>>>>> target without kernel nor bios doesn't really make sense.
>>>>>
>>>>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>>>>
>>>> This is being discussed in http://patchwork.ozlabs.org/patch/262912/ -
>>>> so far Anthony has put a hold on further such changes unfortunately.
>>>>
>>>
>>> This has been an error for more than 6 years, and nobody complained so
>>> far.
>>
>> Neither QOM nor qtest exist for 6 years, so that is not an argument for
>> everything. ;)
>>
>>> I understand that the machines should be testable with qtest, but
>>> such as change has been merged already. Now there is no reason to not
>>> fix this *regression* from version 1.5.
>>
>> Ah, you mean this?
>> http://git.qemu.org/?p=qemu.git;a=commit;h=b332d24a8e1290954029814d09156b06ede358e2
>> Wasn't aware. No objection to exit(1) from my side then.
>>
>> But either way, you shouldn't replace one fprintf() with another
>> fprintf() but instead use our new error_report() if you touch it
>> (without trailing \n then). I've updated my qtest enablement series to
>> use it, v2 handles some more machines.
>>
>>> People should understand that QEMU is not only x86, and that not
>>> everything should be done the x86 way.
>>
>> No need to explain that to me.
> 
> I don't object to adding the exit(1) FWIW.
> 
> But I also think we should think more about having consistent behavior
> across platforms.
> 
> It's unexpected that qemu-system-x86_64 does something and
> qemu-system-mips does something else.
> 
> Maybe -x86_64 should barf is not given anything bootable...

I would surely hope it does? If SeaBIOS is not found (and optionally
!qtest_enabled()), then it should barf, just like -ppc[64]/-sparc[64]
should barf when they don't find OpenBIOS or OHW respectively. When the
firmware has some limited user interaction such as menus or a command
prompt then there is nothing wrong with exposing that to the user.

The difference for some of these arm/mips/ppc/sh4 targets is that we
don't ship any matching firmware out of the box, thus can't rely on its
presence for qtest.

Personally I have found running -x86_64 with some -device (or
-readconfig) can already be quite a useful test case for QOM devices or
for non-destructive migration testing.

By comparison, having -alpha firmware just print "Hello" does not seem
all that useful to me... I wouldn't mind error'ing out without useful
arguments there.

FWIW the Cocoa UI detects a disk image missing from the command line and
prompts for one - yet another behavior. To boot just into the BIOS I
have to specify /dev/null IIRC.

Regards,
Andreas

>> I think Anthony's question was rather whether printing random text to
>> stderr is the best way to address that or whether QEMUMachine could use
>> some this-machine-needs-a-kernel flag that libvirt or someone can access
>> and that could be handled in a central place rather than in each machine
>> as they see fit.
>>
>> But with the release near and no concrete patches, I don't think that's
>> 1.6 material. Question is, do we want test cases based on cleanups that
>> work today in 1.6 and work from there, or do we rather wait 'til after
>> the release and if so, can we get them merged early so that other series
>> can actually be tested with them.
>>
>> Regards,
>> Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg