cros: collect dmesg on USB install failure [infra/infra : main]
Otabek Kasimov (Gerrit)
Otabek Kasimov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Otabek Kasimov
| Auto-Submit | +1 |
| Commit-Queue | +1 |
1 comment
logger.Debugf("Collecting dmesg from USB-booted DUT...")Otabek KasimovThe commit message states that `dmesg` and `lsblk` should be collected *if the installation fails*. However:
1. `dmesg` is being collected here unconditionally, before the OS installation is even attempted.
2. The collection of `lsblk` is completely missing.This logic should likely be moved into the error handling block for `storage.RunInstallOSCommand` (around line 172) so that it only runs on failure and captures the logs from the actual installation attempt. You should also add the missing `lsblk` command there.
That's a good catch; however, since this is strictly intended for collecting dmesg, I don't think it should be moved to `storage.RunInstallOSCommand`. Keeping it separate avoids unnecessary logic complexity and ensures we don't expose additional functionality where it doesn't belong.
Related details
- Bryan Ni
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Bryan Ni (Gerrit)
Bryan Ni voted Code-Review+1![]( "Open in Gerrit")
- Otabek Kasimov
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Otabek Kasimov (Gerrit)
Otabek Kasimov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Otabek Kasimov
| Commit-Queue | +2 |
1 comment
logger.Debugf("Collecting dmesg from USB-booted DUT...")Otabek KasimovThe commit message states that `dmesg` and `lsblk` should be collected *if the installation fails*. However:
1. `dmesg` is being collected here unconditionally, before the OS installation is even attempted.
2. The collection of `lsblk` is completely missing.This logic should likely be moved into the error handling block for `storage.RunInstallOSCommand` (around line 172) so that it only runs on failure and captures the logs from the actual installation attempt. You should also add the missing `lsblk` command there.
That's a good catch; however, since this is strictly intended for collecting dmesg, I don't think it should be moved to `storage.RunInstallOSCommand`. Keeping it separate avoids unnecessary logic complexity and ensures we don't expose additional functionality where it doesn't belong.
resolving as got fresh +1.
Related details
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
LUCI CQ (Gerrit)
LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
cros: collect dmesg on USB install failure
When installing ChromeOS from USB in recovery mode, if the
installation fails, collect dmesg from the DUT.
This helps in diagnosing storage detection issues.
BUG=476190544
TAG=agy
CONV=0b27c66a-4f7d-4717-9064-017a5ddd5ba8
- M go/src/infra/cros/recovery/internal/execs/cros/cros_install_exec.go
Code-Review: +1 by Bryan Ni
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |