| Message ID | 1375567378-5089-1-git-send-email-aurelien@aurel32.net |
|---|---|
| State | New |
| Headers | show |
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. Andreas
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. 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. People should understand that QEMU is not only x86, and that not everything should be done the x86 way.
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 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
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... Regards, Anthony Liguori > > 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
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
On 08/05/2013 07:36 AM, Andreas Färber wrote: > 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. One of these days I'll get around to adding the bits that let you say "boot" on the PALcode console prompt. But in the meantime, I find the ability to boot the bios up to the "Hello" useful as a smoke-test for TCG changes. That said, I can see how it can be confusing for anyone else, it would be more useful to have something integrated with the testsuite instead, so I won't object if we want to print an error message for lack of -kernel or (explicit) -bios command-line arguments. r~
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c index b13750d..f0b8d06 100644 --- a/hw/mips/mips_fulong2e.c +++ b/hw/mips/mips_fulong2e.c @@ -335,7 +335,8 @@ static void mips_fulong2e_init(QEMUMachineInitArgs *args) if ((bios_size < 0 || bios_size > BIOS_SIZE) && !kernel_filename && !qtest_enabled()) { - fprintf(stderr, "qemu: Warning, could not load MIPS bios '%s'\n", bios_name); + fprintf(stderr, "qemu: Could not load MIPS bios '%s'\n", bios_name); + exit(1); } } diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c index 36677cc..7ecb49a 100644 --- a/hw/mips/mips_jazz.c +++ b/hw/mips/mips_jazz.c @@ -178,8 +178,9 @@ static void mips_jazz_init(MemoryRegion *address_space, bios_size = -1; } if ((bios_size < 0 || bios_size > MAGNUM_BIOS_SIZE) && !qtest_enabled()) { - fprintf(stderr, "qemu: Warning, could not load MIPS bios '%s'\n", + fprintf(stderr, "qemu: Could not load MIPS bios '%s'\n", bios_name); + exit(1); } /* Init CPU internal devices */ diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c index f56f34f..5726650 100644 --- a/hw/mips/mips_malta.c +++ b/hw/mips/mips_malta.c @@ -1009,8 +1009,9 @@ void mips_malta_init(QEMUMachineInitArgs *args) if ((bios_size < 0 || bios_size > BIOS_SIZE) && !kernel_filename && !qtest_enabled()) { fprintf(stderr, - "qemu: Warning, could not load MIPS bios '%s', and no -kernel argument was specified\n", + "qemu: Could not load MIPS bios '%s', and no -kernel argument was specified\n", bios_name); + exit(1); } } /* In little endian mode the 32bit words in the bios are swapped, diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c index fea1a15..f7ec179 100644 --- a/hw/mips/mips_mipssim.c +++ b/hw/mips/mips_mipssim.c @@ -192,8 +192,9 @@ mips_mipssim_init(QEMUMachineInitArgs *args) if ((bios_size < 0 || bios_size > BIOS_SIZE) && !kernel_filename) { /* Bail out if we have neither a kernel image nor boot vector code. */ fprintf(stderr, - "qemu: Warning, could not load MIPS bios '%s', and no -kernel argument was specified\n", + "qemu: Could not load MIPS bios '%s', and no -kernel argument was specified\n", filename); + exit(1); } else { /* We have a boot vector start address. */ env->active_tc.PC = (target_long)(int32_t)0xbfc00000;
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> --- hw/mips/mips_fulong2e.c | 3 ++- hw/mips/mips_jazz.c | 3 ++- hw/mips/mips_malta.c | 3 ++- hw/mips/mips_mipssim.c | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-)