| Message ID | 4D416342.6080001@redhat.com |
|---|---|
| State | New |
| Headers | show |
Kevin Wolf <kwolf@redhat.com> writes: > Am 27.01.2011 13:10, schrieb Markus Armbruster: >> Consider -drive if=scsi,index=12,... >> >> Before the commit, index=12 meant bus=1,unit=5. Example: >> >> [...] >> >> Two scsi-buses, and scsi1-cd5 with scsi-id 5 is on the second one, >> i.e. bus=1, unit=5. >> >> After the commit, it means bus=0,unit=12. The drive is created, but not >> the guest device. That's because lsi53c895a supports only 7 units >> (LSI_MAX_DEVS), and scsi_bus_legacy_handle_cmdline() ignores drives with >> unit numbers exceeding that limit. Example: >> >> [...] >> >> One scsi-bus, and scsi1-cd5 nowhere to be found. >> >> I'd call this a regression. >> >> What now? > > That's a really good question. We could do something like this: > > --- a/blockdev.c > +++ b/blockdev.c > @@ -192,7 +192,7 @@ DriveInfo *drive_init(QemuOpts *opts, int > default_to_scsi) > max_devs = MAX_IDE_DEVS; > } else if (!strcmp(buf, "scsi")) { > type = IF_SCSI; > - max_devs = MAX_SCSI_DEVS; > + max_devs = 7; > } else if (!strcmp(buf, "floppy")) { > type = IF_FLOPPY; > max_devs = 0; > > That's very obviously not much more than a hack, but I don't think > blockdev.c can get the real number easily (please prove me wrong). With > this hack, we would get the old behaviour for -drive (which doesn't use > any other controller anyway) and you can still use -device to attach > more devices to a non-lsi bus. The real number is 7, because it's always been 7 :) Okay, back to serious. drive_init() with IF_SCSI is tied to scsi_bus_legacy_handle_cmdline() and its callers. The callers are SCSI controller device initialization functions. The controller has a per bus unit limit. And that's the real number for that bus. But drive_init() doesn't know which device is going to provide a given bus number! It shouldn't know. Perhaps it even can't know. We have two callers now: lsi_scsi_init(), limit 7 (LSI_MAX_DEVS), and esp_init1(), limit 7 (ESP_MAX_DEVS). Let's worry about it when we add a caller with a different limit.
Hi, > } else if (!strcmp(buf, "scsi")) { > type = IF_SCSI; > - max_devs = MAX_SCSI_DEVS; > + max_devs = 7; > That's very obviously not much more than a hack, but I don't think > blockdev.c can get the real number easily (please prove me wrong). With > this hack, we would get the old behaviour for -drive (which doesn't use > any other controller anyway) and you can still use -device to attach > more devices to a non-lsi bus. Looks sensible to me. scsi controllers with more than 7 devs (megasas and whatever else might be coming, virtio?) can't be added via if=scsi legacy syntax and must use -blockdev and -device anyway. We probably want add a comment explaining this though. cheers, Gerd
On 01/27/2011 08:26 PM, Gerd Hoffmann wrote: > Hi, > >> } else if (!strcmp(buf, "scsi")) { >> type = IF_SCSI; >> - max_devs = MAX_SCSI_DEVS; >> + max_devs = 7; > >> That's very obviously not much more than a hack, but I don't think >> blockdev.c can get the real number easily (please prove me wrong). With >> this hack, we would get the old behaviour for -drive (which doesn't use >> any other controller anyway) and you can still use -device to attach >> more devices to a non-lsi bus. > > Looks sensible to me. scsi controllers with more than 7 devs (megasas > and whatever else might be coming, virtio?) can't be added via if=scsi > legacy syntax and must use -blockdev and -device anyway. > > We probably want add a comment explaining this though. > Yes please. It's a bit non-obvious that 'scsi' means in fact 'lsi'. As long as there a way of specifying more than 8 devs I'm fine with that patch. Cheers, Hannes
--- a/blockdev.c +++ b/blockdev.c @@ -192,7 +192,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) max_devs = MAX_IDE_DEVS; } else if (!strcmp(buf, "scsi")) { type = IF_SCSI; - max_devs = MAX_SCSI_DEVS; + max_devs = 7; } else if (!strcmp(buf, "floppy")) { type = IF_FLOPPY;