From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58375) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z61gV-0000c4-IW for qemu-devel@nongnu.org; Fri, 19 Jun 2015 15:10:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z61gT-0000Sx-KT for qemu-devel@nongnu.org; Fri, 19 Jun 2015 15:10:39 -0400 Message-ID: <5584691F.4040801@redhat.com> Date: Fri, 19 Jun 2015 21:10:23 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <55845CBF.8010906@redhat.com> <558463E1.5000507@redhat.com> In-Reply-To: <558463E1.5000507@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] another isa-fdc problem... List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow , qemu devel list Cc: Kevin Wolf , =?UTF-8?B?SsOhbiBUb21rbw==?= , Markus Armbruster , qemu-block@nongnu.org, Eduardo Habkost On 06/19/15 20:48, John Snow wrote: >=20 >=20 > On 06/19/2015 02:17 PM, Laszlo Ersek wrote: >> With Eduardo's recent patch (473a49460db0a90bfda046b8f3662b49f94098eb)= , >> q35 machtypes earlier than 2.4 work as expected. Also, pc-q35-2.4 work= s >> fine in the default case (no board-default FDC, which is what we want)= , >> and the traditional option "-drive if=3Dfloppy,..." also works as expe= cted. >> >> However, J=C3=A1n noticed that on pc-q35-2.4, the "canonical" >> frontend/backend option pair >> >> -device isa-fdc,driveA=3Ddrive-fdc0-0-0 \ >> -drive file=3D...,if=3Dnone,id=3Ddrive-fdc0-0-0,format=3Draw >> >> breaks guest OS detection of the FDC. Markus has now verified that the >> qtree looks good (thanks for that), but the guest still doesn't notice >> the FDC. >> >> In and >> I wrote: >> >>> If you create an isa-fdc *outside* of the board init code, ie. outsid= e >>> of the pc_q35_init() function, then in the following call chain: >>> >>> pc_q35_init() >>> pc_basic_device_init() >>> pc_cmos_init() >>> ... >>> rtc_set_memory(s, 0x10, val); >>> >>> the value stored at 0x10 in CMOS will not reflect the floppy. This is >>> probably what trips up a guest OS -- they look to the CMOS for seeing >>> floppy presence. >>> >>> http://wiki.osdev.org/CMOS#Register_0x10 >>> http://www.osdever.net/tutorials/view/detecting-floppy-drives >>> >>> In the guest kernel, "drivers/block/floppy.c" seems to be a heavy use= r >>> of CMOS too. >>> >>> [...] >>> >>> BTW I did know (and document, in >>> fd53c87cf6651b0dfe9f5107cfe77d2f697bd4f6) that pc_cmos_init() would >>> get NULL for "floppy" (ie. FDC) if the board-default FDC was absent. >>> What I didn't expect was that this would prevent a guest OS from >>> seeing an FDC (without special module parameters) that was created >>> differently. >>> >>> [...] >> >> Thus, I didn't regress earlier machine types, nor earlier command line= s >> with the new pc-q35-2.4 machine type, but I also didn't get right the >> behavior for the "canonical" options that libvirt will want to use. >> Markus suggested a way to fix that: >> >> off the cuff: the floppy code needs to move from pc_cmos_init() to >> pc_cmos_init_late(), and find the isa-fdc regardless of how it was >> added ... may have to walk qom data structures >> >> But before I try to do that, I looked at any preexistent test cases. >> >> Sure enough, tests/fdc-test.c catches this (in the "cmos" test), *if* >> you hack it to start QEMU with -M q35. Therefore my first question is >> what the best practice is for running a set of tests on different >> machine types. >> >> QOSState / set_context() / qtest_pc_vboot() seem relevant (example: >> "ahci-test.c"), but >> - they also appear quite heavy weight, >=20 > They're not /that/ heavy. They just set up an allocator and enable IRQ > intercepts. If you don't use migrate or the allocator, the overhead > should be pretty low. Just a lot of va_args shenanigans to make them > easy to shoehorn into lots of scenarios. >=20 >> - I'd lose (or have to open-code) the default options from qtest_init(= ), >=20 > qtest_pc_boot > ..qtest_pc_vboot > ....qtest_vboot > ......qtest_start > ........qtest_init >=20 > You keep the default args when going through this chain, unless I am > misunderstanding you. You understood right; I didn't dig down this deep. Thanks. >> - and I'd like to avoid: >> - starting a separate qemu process for each single test, >> - starting the necessary minimum number of qemu processes *in >> parallel*. >> >> So some way would be necessary where I can add a bunch of test cases f= or >> different machine types, and gtester would stop the old qemu instance >> and start the new qemu instance between tests when it notices that the >> machine type changes from the previous test. I vaguely recall having >> done something with GTester fixtures in the opts-visitor test, but it'= s >> foggy. So, what's the best practice for this? Of course I could just >> share data between tests, but that doesn't appear nice. >> >=20 > Are you sure you want to share QEMU instances between tests until we tr= y > to change the boot options? I didn't really consider support for this > inside of unit tests because I was encouraged to do full > boot-up/shut-down so that each test would be independent. fdc-test runs 13 tests between qtest_start() and qtest_end(). I thought that was a nice performance property, and many other tests do the same. However, if individual startup is okay for the AHCI tester (and you were actually encouraged to do that), then I guess I could just follow suit. > I guess you want some lazy-eval way of spinning up instances only if we > need them, and sharing those instances between tests? Nothing like that > exists within qtest today, but if you can sort your tests by machine > type, then: >=20 > typedef struct QOSState { > QTestState *qts; > + QMachineType type; > QOSOps *ops; > } QOSState; >=20 > const char *machine_lookup(enum machinetype) > { > switch (machinetype) { > case Q35_2_4: > return "pc-q35-2.4" > ... > } > } >=20 > QOSState *fdc_get_machine(enum machinetype) > { > static QOSState *machine; >=20 > if (machine && machine->type =3D=3D machinetype) { > return machine; > } else { > if (machine) { > qtest_pc_shutdown(machine); > } > machine =3D qtest_pc_boot("blah blah my defaults here -M %s", > machine_lookup(machinetype)); > return machine; > } > } >=20 > Then just call fdc_get_machine a bunch. You can cram all your defaults > in fdc_get_machine without bothering the rest of the infrastructure. That's exactly what I had in mind, with "share data between tests" -- but maybe this would lead to my excommunication from qemu-devel :) (Also I think I would not modify QOSState itself.) > Not sure if there's a nicer way to do it, I wouldn't lose too much slee= p > over design purity in qtests, especially so close to freeze. >=20 >> My other question: we're past the 2.4 soft freeze; if I can't fix this >> until the hard freeze, how big a problem is that? The "new" way won't = be >> available in 2.4, but the "old" ways should work. >> >=20 > I'm willing to help review and pull things in until fairly late, as lon= g > as Peter Maydell doesn't yell at me for doing so. Thanks! >> (... Oh geez why did I touch the FDC. :() >> >=20 > You're stuck now! Thanks for twisting the dagger... I'm swamped by other "more important" / "more urgent" stuff; I only picked up the "eliminate FDC" thing because I whined about the FDC, and wanted to avoid an impression that "Laszlo can only whine and never do something about it". I guess I must resist the urge to whine next time. Is it perhaps safer to revert those five patches for 2.4, and let me reboot them (... if I don't change my mind... :)) after 2.4 is out? To be clear, this has nothing to do with my willingness, and everything with my capacity. You may have noticed that I posted the v7 PXB series yesterday ^W today at 04:40 AM (CEST)... Thanks Laszlo