On Tue, Jun 20, 2017 at 02:42:15PM +0200, Paolo Bonzini wrote:
On 19/06/2017 23:55, Roman Kagan wrote:
On Mon, Jun 19, 2017 at 06:21:09PM +0200, Paolo Bonzini wrote:
The introduction of REPORT LUNS caused a potential regression when many disks are present on the same SCSI target. If LUN0 is processed too late, SeaBIOS might run out of memory and fail to register it.
There are two kinds of problems:
QEMU presents the results of REPORT LUNS in the opposite order to the "-device" command-line options. It's likely that LUN 0 will be first in the command-line, and thus will be processed last by SeaBIOS.
QEMU registers all LUNs in each target first. It's thus possible that LUN 0 of target 1 is processed after many LUNs of target 0 and not be able to allocate memory.
Am I correct that this is not two problems but one: the recently added REPORT LUNS enumeration made more disks visible to SeaBIOS, so that their control structures may not all fit in the limited available memory, and their unfortunate ordering may result in the most relevant drives falling out?
Yes, I listed them separately because you could fix the first by e.g. sorting the REPORT LUNS output.
But does the LUN processing order matter per se? Doesn't fixing it only change which LUNs are more likely to fall out?
To fix both, the SCSI scans are modified to operate in two phases; LUN 0 is scanned before the REPORT LUNS command is sent. A possible future enhancement is to remember the results of the scan in a 32-byte bitmap, and skip the REPORT LUNS command during the second phase.
When multiple SCSI controllers are found, it's still possible to have a regression because they are scanned in parallel.
I think there's a simpler but more reliable solution. One can safely assume that the operating systems that use BIOS to access hard drives aren't interested in that many LUNs being visible. So we can just skip calling scsi_drive_setup() and return non-zero from the respective add_lun functions if the lun number is above some predefined number (say, 2 or 8) and it doesn't appear on the fw_cfg bootorder list, e.g.
I'm not sure about the reliability; in practice this is a corner case and there is room for many LUNs in the upper memory blocks, and ignoring some LUNs (but not all of them) may produce unexpected results on some configurations.
I'm not sure I get the point. Basically it all turns around which LUNs to ignore: originally it was "higher LUNs in lower targets win", with your patch it's "LUN 0 in all targets win", with what I propose it's "LUNs up to N and bootable ones in all targets win". Am I missing anything?
[BTW what are those unexpected results that may arise from ignoring some LUNs, other than being unable to use them in guest DOS?]
I also kind of liked the idea of reusing the result of the LUN0 scan to speed up virtio-scsi boot. :)
Yes that's a cool trick but it wouldn't be needed without two-phase enumeration, would it? ;)
Roman.