Re: [Qemu-devel] [PATCH 5/8] ipmi: use a file to load SDRs

[Marcel Apfelbaum]

On 02/09/2016 02:13 PM, Cédric Le Goater wrote:
> The IPMI BMC simulator populates the sdr/sensor tables with a minimal
> set of entries (Watchdog). But some qemu platforms might want to use
> extra entries for their custom needs.
>
> This patch modifies slighty the initializing routine to take into
> account a larger set read from a file. The name of the file to use is
> defined through a new 'sdr' property of the simulator device.
>
> Signed-off-by: Cédric Le Goater <address@hidden>
> ---
>   hw/ipmi/ipmi_bmc_sim.c | 19 +++++++++++++++----
>   1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index aff818cf22ab..69318eb6b556 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -27,6 +27,7 @@
>   #include "qemu/timer.h"
>   #include "hw/ipmi/ipmi.h"
>   #include "qemu/error-report.h"
> +#include "hw/loader.h"
>
>   #define IPMI_NETFN_CHASSIS            0x00
>
> @@ -208,6 +209,7 @@ struct IPMIBmcSim {
>       IPMISel sel;
>       IPMISdr sdr;
>       IPMISensor sensors[MAX_SENSORS];
> +    char *sdr_filename;
>
>       /* Odd netfns are for responses, so we only need the even ones. */
>       const IPMINetfn *netfns[MAX_NETFNS / 2];
> @@ -1708,24 +1710,32 @@ static void ipmi_sdr_init(IPMIBmcSim *ibs)
>       size_t sdrs_size;
>       uint8_t *sdrs;
>
> -    sdrs_size = sizeof(init_sdrs);
> -    sdrs = init_sdrs;
> +    if (!ibs->sdr_filename ||
> +        !g_file_get_contents(ibs->sdr_filename, (gchar **) &sdrs, &sdrs_size,
> +                            NULL)) {

Hi,

If the file exists but you cannot read it you may want at least to
warn the user. He may think the contents are read from the file.

Other than that and the change of the property name (as suggested in this mail 
thread

Reviewed-by: Marcel Apfelbaum <address@hidden>

Thanks,
Marcel


> +        sdrs_size = sizeof(init_sdrs);
> +        sdrs = init_sdrs;
> +    }
>
>       for (i = 0; i < sdrs_size; i += len) {
>           struct ipmi_sdr_header *sdrh;
>
>           if (i + IPMI_SDR_HEADER_SIZE > sdrs_size) {
>               error_report("Problem with recid 0x%4.4x", i);
> -            return;
> +            break;
>           }
>           sdrh = (struct ipmi_sdr_header *) &sdrs[i];
>           len = ipmi_sdr_length(sdrh);
>           if (i + len > sdrs_size) {
>               error_report("Problem with recid 0x%4.4x", i);
> -            return;
> +            break;
>           }
>           sdr_add_entry(ibs, sdrh, len, NULL);
>       }
> +
> +    if (sdrs != init_sdrs) {
> +        g_free(sdrs);
> +    }
>   }
>
>   static const VMStateDescription vmstate_ipmi_sim = {
> @@ -1796,6 +1806,7 @@ static void ipmi_sim_realize(DeviceState *dev, Error 
> **errp)
>   }
>
>   static Property ipmi_sim_properties[] = {
> +    DEFINE_PROP_STRING("sdr", IPMIBmcSim, sdr_filename),
>       DEFINE_PROP_END_OF_LIST(),
>   };