[PATCH] pretty boot menu entry for cdrom drives
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- src/util.h | 1 + src/boot.c | 7 +++++++ src/cdrom.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/src/util.h b/src/util.h index 7a23b518fc..6dd080f673 100644 --- a/src/util.h +++ b/src/util.h @@ -50,6 +50,7 @@ struct disk_op_s; int cdemu_process_op(struct disk_op_s *op); void cdrom_prepboot(void); int cdrom_boot(struct drive_s *drive_g); +char *cdrom_media_info(struct drive_s *drive_g); // clock.c void clock_setup(void); diff --git a/src/boot.c b/src/boot.c index ff705fd47f..80bcc13535 100644 --- a/src/boot.c +++ b/src/boot.c @@ -395,6 +395,13 @@ boot_add_hd(struct drive_s *drive, const char *desc, int prio) void boot_add_cd(struct drive_s *drive, const char *desc, int prio) { + if (GET_GLOBAL(PlatformRunningOn) & PF_QEMU) { + // We want short boot times. But on physical hardware even + // the test unit ready can take several seconds. So do media + // access on qemu only, where we know it will be fast. + char *extra = cdrom_media_info(drive); + desc = znprintf(MAXDESCSIZE, "%s (%s)", desc, extra); + } bootentry_add(IPL_TYPE_CDROM, defPrio(prio, DefaultCDPrio) , (u32)drive, desc); } diff --git a/src/cdrom.c b/src/cdrom.c index 828fb3b842..b4c36622ae 100644 --- a/src/cdrom.c +++ b/src/cdrom.c @@ -274,3 +274,54 @@ cdrom_boot(struct drive_s *drive) return 0; } + +// go figure some cdrom information, for a pretty boot menu entry. +char* +cdrom_media_info(struct drive_s *drive) +{ + ASSERT32FLAT(); + + struct disk_op_s dop; + memset(&dop, 0, sizeof(dop)); + dop.drive_fl = drive; + + int ret = scsi_is_ready(&dop); + if (ret) + return "empty"; + + // Read the Boot Record Volume Descriptor + u8 buffer[CDROM_SECTOR_SIZE]; + dop.command = CMD_READ; + dop.lba = 0x11; + dop.count = 1; + dop.buf_fl = buffer; + ret = process_op(&dop); + if (ret) + return "read error"; + + // Is it bootable? + if (buffer[0]) + return "not bootable"; + if (strcmp((char*)&buffer[1], "CD001\001EL TORITO SPECIFICATION") != 0) + return "not bootable"; + + // Read the Primary Volume Descriptor + dop.command = CMD_READ; + dop.lba = 0x10; + dop.count = 1; + dop.buf_fl = buffer; + ret = process_op(&dop); + if (ret) + return "read error"; + + // Read volume id, trim trailing spaces + char *volume = znprintf(30, "%s", buffer + 40); + char *h = volume + strlen(volume); + while (h > volume) { + h--; + if (*h != ' ') + break; + *h = 0; + } + return volume; +} -- 2.9.3
On Fri, Sep 21, 2018 at 09:48:01AM +0200, Gerd Hoffmann wrote:
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- src/util.h | 1 + src/boot.c | 7 +++++++ src/cdrom.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+)
diff --git a/src/util.h b/src/util.h index 7a23b518fc..6dd080f673 100644 --- a/src/util.h +++ b/src/util.h @@ -50,6 +50,7 @@ struct disk_op_s; int cdemu_process_op(struct disk_op_s *op); void cdrom_prepboot(void); int cdrom_boot(struct drive_s *drive_g); +char *cdrom_media_info(struct drive_s *drive_g);
// clock.c void clock_setup(void); diff --git a/src/boot.c b/src/boot.c index ff705fd47f..80bcc13535 100644 --- a/src/boot.c +++ b/src/boot.c @@ -395,6 +395,13 @@ boot_add_hd(struct drive_s *drive, const char *desc, int prio) void boot_add_cd(struct drive_s *drive, const char *desc, int prio) { + if (GET_GLOBAL(PlatformRunningOn) & PF_QEMU) { + // We want short boot times. But on physical hardware even + // the test unit ready can take several seconds. So do media + // access on qemu only, where we know it will be fast. + char *extra = cdrom_media_info(drive); + desc = znprintf(MAXDESCSIZE, "%s (%s)", desc, extra); + } bootentry_add(IPL_TYPE_CDROM, defPrio(prio, DefaultCDPrio) , (u32)drive, desc); } diff --git a/src/cdrom.c b/src/cdrom.c index 828fb3b842..b4c36622ae 100644 --- a/src/cdrom.c +++ b/src/cdrom.c @@ -274,3 +274,54 @@ cdrom_boot(struct drive_s *drive)
return 0; } + +// go figure some cdrom information, for a pretty boot menu entry. +char* +cdrom_media_info(struct drive_s *drive) +{ + ASSERT32FLAT(); + + struct disk_op_s dop; + memset(&dop, 0, sizeof(dop)); + dop.drive_fl = drive; + + int ret = scsi_is_ready(&dop); + if (ret) + return "empty"; + + // Read the Boot Record Volume Descriptor + u8 buffer[CDROM_SECTOR_SIZE]; + dop.command = CMD_READ; + dop.lba = 0x11; + dop.count = 1; + dop.buf_fl = buffer; + ret = process_op(&dop); + if (ret) + return "read error"; + + // Is it bootable? + if (buffer[0]) + return "not bootable"; + if (strcmp((char*)&buffer[1], "CD001\001EL TORITO SPECIFICATION") != 0) + return "not bootable"; + + // Read the Primary Volume Descriptor + dop.command = CMD_READ; + dop.lba = 0x10; + dop.count = 1; + dop.buf_fl = buffer; + ret = process_op(&dop); + if (ret) + return "read error"; + + // Read volume id, trim trailing spaces + char *volume = znprintf(30, "%s", buffer + 40); + char *h = volume + strlen(volume); + while (h > volume) { + h--; + if (*h != ' ') + break; + *h = 0; + } + return volume; +}
Thanks. In general, looks fine to me. Some minor comments: If there's a read error or non-bootable device, shouldn't it just revert to the original description? There's a nullTrailingSpace() function to remove trailing spaces. -Kevin
Thanks. In general, looks fine to me.
Some minor comments:
If there's a read error or non-bootable device, shouldn't it just revert to the original description?
What about "empty" for drives not ready? Drop that too? I think we should be consistent here: Either be verbose and report "empty" / "read error" / "not-bootable" status in case we figure we can't boot from the drive for one of the listed reasons. Or be quiet and just report the volume label for bootable drives and nothing otherwise. I've also toyed with the idea to use a higher default priority for non-bootable cdrom drives, so with multiple cdroms and no explicit boot order the bootable would be listed first and used by default. What do you think about this?
There's a nullTrailingSpace() function to remove trailing spaces.
Ah, good, I'll use that. cheers, Gerd
On Wed, Sep 26, 2018 at 07:03:37AM +0200, Gerd Hoffmann wrote:
Thanks. In general, looks fine to me.
Some minor comments:
If there's a read error or non-bootable device, shouldn't it just revert to the original description?
What about "empty" for drives not ready? Drop that too?
Well, if I saw: 4. DVD/CD [ata1-0: QEMU DVD-ROM ATAPI-4 DVD/CD] (empty) I don't think I'd know what "empty" meant. Also, it's a little odd to have a C function sometimes return a dynamically allocated string and sometimes return a constant string. That said, I don't have a strong opinion. -Kevin
Kevin O'Connor wrote:
it's a little odd to have a C function sometimes return a dynamically allocated string and sometimes return a constant string.
Gerd, please don't do that. Sure, maybe nothing is ever free()d, but that's still very poor practice. Don't spread it. //Peter
participants (3)
-
Gerd Hoffmann -
Kevin O'Connor -
Peter Stuge