From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49932) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVMY9-00064A-Lk for qemu-devel@nongnu.org; Mon, 15 Feb 2016 12:03:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aVMY3-0002Af-TS for qemu-devel@nongnu.org; Mon, 15 Feb 2016 12:03:01 -0500 Received: from e06smtp08.uk.ibm.com ([195.75.94.104]:50988) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVMY3-0002AJ-9A for qemu-devel@nongnu.org; Mon, 15 Feb 2016 12:02:55 -0500 Received: from localhost by e06smtp08.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 15 Feb 2016 17:02:52 -0000 Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id 3A4A517D8066 for ; Mon, 15 Feb 2016 17:03:08 +0000 (GMT) Received: from d06av08.portsmouth.uk.ibm.com (d06av08.portsmouth.uk.ibm.com [9.149.37.249]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u1FH2o9q63439030 for ; Mon, 15 Feb 2016 17:02:50 GMT Received: from d06av08.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av08.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u1FH2omx022398 for ; Mon, 15 Feb 2016 10:02:50 -0700 References: <1455020010-17532-1-git-send-email-clg@fr.ibm.com> <1455020010-17532-6-git-send-email-clg@fr.ibm.com> <56C04402.2060601@gmail.com> From: =?UTF-8?Q?C=c3=a9dric_Le_Goater?= Message-ID: <56C204B8.9060603@fr.ibm.com> Date: Mon, 15 Feb 2016 18:02:48 +0100 MIME-Version: 1.0 In-Reply-To: <56C04402.2060601@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 5/8] ipmi: use a file to load SDRs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: marcel@redhat.com, Corey Minyard Cc: "Michael S. Tsirkin" , qemu-devel@nongnu.org, Greg Kurz On 02/14/2016 10:08 AM, Marcel Apfelbaum wrote: > 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 >> --- >> 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. In fact, I did in an early version of the patch and then I removed it because I thought qemu did not have to complain for not fatal errors. I will add a error_report(). Thanks, C. > Other than that and the change of the property name (as suggested in this mail thread > > Reviewed-by: Marcel Apfelbaum > > 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(), >> }; >> >> >