Re: [PATCH v4 1/2] hw/uefi: add "info firmware-log" + "query-firmware-log" monitor commands

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Oct 15, 2025 at 09:41:15AM +0200, Markus Armbruster wrote:
> Gerd Hoffmann <kraxel@redhat.com> writes:
> 
> >   Hi,
> >
> >> > +static void handle_ovmf_log_range(GString *out,
> >> > +                                  dma_addr_t start,
> >> > +                                  dma_addr_t end,
> >> > +                                  Error **errp)
> >> > +{
> >> > +    g_autofree char *buf = NULL;
> >> 
> >> Left-over variable - unfortunately gcc doens't warn about unused
> >> vars when g_autofree is present :-(
> >
> > Fixed.
> >
> >> > +{ 'struct': 'FirmwareLog',
> >> > +  'data': { '*version': 'str',
> >> > +            '*log': 'str' } }
> >> 
> >> Having version be optional makes sense, but what is the semantics for
> >> 'log' being optional. If the log is unavailable, wouldn't we be better
> >> raising an error ?
> >
> > That is what the current implementation actually does.  I change it.
> 
> Drawback: when some Firmware provides version, but not log, the command
> can't give us the version, because it fails.  Do we care?

This command is literally called 'query-firmware-log' though. From this
I consider the log to be the important deliverable. If not available then
I would expect an error from such a named command. If we want to provide
non-log related info about the firmware when no log is available, then we
should rename the command to "query-firmware-info" instead.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|