| Message ID | 1389716144-31076-2-git-send-email-bsd@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 2014-01-14 at 22:37 +0530, Bandan Das wrote: > Ccing Markus for the *_once macros > > Alex Williamson <alex.williamson@redhat.com> writes: > > > On Tue, 2014-01-14 at 21:45 +0530, Bandan Das wrote: > >> If the device rom can't be read, report an error to the > >> user. The guest might try to read the rom contents more than > >> once, so introduce macros that print a message only once and > >> not clutter up the console. This is to alert the user > >> that the device has a bad state that is causing rom read > >> failure or option rom loading has been disabled from the device > >> boot menu (among other reasons). > >> > >> Signed-off-by: Bandan Das <bsd@redhat.com> > >> --- > >> hw/misc/vfio.c | 7 +++++++ > >> include/qemu/error-report.h | 20 ++++++++++++++++++++ > >> 2 files changed, 27 insertions(+) > >> > >> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c > >> index 9aecaa8..e5b2826 100644 > >> --- a/hw/misc/vfio.c > >> +++ b/hw/misc/vfio.c > >> @@ -1125,6 +1125,13 @@ static void vfio_pci_load_rom(VFIODevice *vdev) > >> vdev->rom_offset = reg_info.offset; > >> > >> if (!vdev->rom_size) { > >> + error_report_once("vfio-pci: Cannot read device rom at " > >> + "%04x:%02x:%02x.%x\n", > >> + vdev->host.domain, vdev->host.bus, vdev->host.slot, > >> + vdev->host.function); > >> + error_printf_once("Device option ROM contents are probably invalid " > >> + "(check dmesg).\nSkip option ROM probe with rombar=0, " > >> + "or load from file with romfile=\n"); > >> return; > >> } > >> > >> diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h > >> index 3b098a9..7d24e4c 100644 > >> --- a/include/qemu/error-report.h > >> +++ b/include/qemu/error-report.h > >> @@ -43,4 +43,24 @@ void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2); > >> const char *error_get_progname(void); > >> extern bool enable_timestamp_msg; > >> > >> +#define error_printf_once(fmt, ...) \ > >> +({ \ > >> + static bool __printf_once; \ > >> + \ > >> + if (!__printf_once) { \ > >> + __printf_once = true; \ > >> + error_printf(fmt, ##__VA_ARGS__); \ > >> + } \ > >> +}) \ > >> + > >> +#define error_report_once(fmt, ...) \ > >> +({ \ > >> + static bool __report_once; \ > >> + \ > >> + if (!__report_once) { \ > >> + __report_once = true; \ > >> + error_report(fmt, ##__VA_ARGS__); \ > >> + } \ > >> +}) \ > >> + > >> #endif > > > > Why do we need these if patch 2/2 comes along and only calls > > vfio_pci_load_rom() once? If we do need these, they should be a > > separate patch. Thanks, > > I was in and out on this until I decided to include it for cases > where we vfio assign a number of functions from the same card - if rom > loading fails for one, it will most probably fail for others as > well and this will make sure to print it just once at bootup. > However, this also means that it will print once for unrelated assignments > too, I kind of half-convinced myself that that's probably ok :) > > Would you rather have this get printed for each assigned device if loading > fails ? The error_report is going to list a specific device, so yes, it's probably best to be explicit about all the devices that are having problems. Thanks, Alex
diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c index 9aecaa8..e5b2826 100644 --- a/hw/misc/vfio.c +++ b/hw/misc/vfio.c @@ -1125,6 +1125,13 @@ static void vfio_pci_load_rom(VFIODevice *vdev) vdev->rom_offset = reg_info.offset; if (!vdev->rom_size) { + error_report_once("vfio-pci: Cannot read device rom at " + "%04x:%02x:%02x.%x\n", + vdev->host.domain, vdev->host.bus, vdev->host.slot, + vdev->host.function); + error_printf_once("Device option ROM contents are probably invalid " + "(check dmesg).\nSkip option ROM probe with rombar=0, " + "or load from file with romfile=\n"); return; } diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h index 3b098a9..7d24e4c 100644 --- a/include/qemu/error-report.h +++ b/include/qemu/error-report.h @@ -43,4 +43,24 @@ void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2); const char *error_get_progname(void); extern bool enable_timestamp_msg; +#define error_printf_once(fmt, ...) \ +({ \ + static bool __printf_once; \ + \ + if (!__printf_once) { \ + __printf_once = true; \ + error_printf(fmt, ##__VA_ARGS__); \ + } \ +}) \ + +#define error_report_once(fmt, ...) \ +({ \ + static bool __report_once; \ + \ + if (!__report_once) { \ + __report_once = true; \ + error_report(fmt, ##__VA_ARGS__); \ + } \ +}) \ + #endif
If the device rom can't be read, report an error to the user. The guest might try to read the rom contents more than once, so introduce macros that print a message only once and not clutter up the console. This is to alert the user that the device has a bad state that is causing rom read failure or option rom loading has been disabled from the device boot menu (among other reasons). Signed-off-by: Bandan Das <bsd@redhat.com> --- hw/misc/vfio.c | 7 +++++++ include/qemu/error-report.h | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+)