On 7/17/20 10:03 AM, Thomas Huth wrote: > On 17/07/2020 09.48, Philippe Mathieu-Daudé wrote: >> +Thomas > >> On 7/16/20 10:56 PM, Havard Skinnemoen wrote: >>> On Wed, Jul 15, 2020 at 1:54 PM Havard Skinnemoen >>> <hskinnem...@google.com> wrote: >>>> >>>> On Wed, Jul 15, 2020 at 3:57 AM Philippe Mathieu-Daudé <f4...@amsat.org> >>>> wrote: >>>>> >>>>> On 7/15/20 11:00 AM, Markus Armbruster wrote: >>>>>> Now my point. Why first make up user configuration, then use that to >>>>>> create a BlockBackend, when you could just go ahead and create the >>>>>> BlockBackend? >>>>> >>>>> CLI issue mostly. >>>>> >>>>> We can solve it similarly to the recent "sdcard: Do not allow invalid SD >>>>> card sizes" patch: >>>>> >>>>> if (!dinfo) { >>>>> error_setg(errp, "Missing SPI flash drive"); >>>>> error_append_hint(errp, "You can use a dummy drive using:\n"); >>>>> error_append_hint(errp, "-drive if=mtd,driver=null-co," >>>>> "read-ones=on,size=64M\n); >>>>> return; >>>>> } >>>>> >>>>> having npcm7xx_connect_flash() taking an Error* argument, >>>>> and MachineClass::init() call it with &error_fatal. >>>> >>>> Erroring out if the user specifies a configuration that can't possibly >>>> boot sounds good to me. Better than trying to come up with defaults >>>> that are still not going to result in a bootable system. >>>> >>>> For testing recovery paths, I think it makes sense to explicitly >>>> specify a null device as you suggest. >>> >>> Hmm, one problem. qom-test fails with >>> >>> qemu-system-aarch64: Missing SPI flash drive >>> You can add a dummy drive using: >>> -drive if=mtd,driver=null-co,read-zeroes=on,size=32M >>> Broken pipe >>> /usr/local/google/home/hskinnemoen/qemu/for-upstream/tests/qtest/libqtest.c:166: >>> kill_qemu() tried to terminate QEMU process but encountered exit >>> status 1 (expected 0) >>> ERROR qom-test - too few tests run (expected 68, got 7) >>> >>> So it looks like we might need a different solution to this, unless we >>> want to make generic tests more machine-aware... > > I didn't follow the other mails in this thread, but what we usually do > in such a case: Add a "if (qtest_enabled())" check to the device or the > machine to ignore the error if it is running in qtest mode.
Hmm I'm not sure it works in this case. We could do: if (!dinfo) { if (qtest) { /* create null drive for qtest */ opts = ...; dinfo = drive_new(opts, IF_MTD, &error_abort); } else { /* teach user to use proper CLI */ error_setg(errp, "Missing SPI flash drive"); error_append_hint(errp, "You can use a dummy drive using:\n"); error_append_hint(errp, "-drive if=mtd,driver=null-co," "read-ones=on,size=64M\n); } } But I'm not sure Markus will enjoy it :) Markus, any better idea about how to handle that with automatic qtests? Thanks :) Phil. > > Thomas > >