On 9/9/20 9:35 AM, Daniel P. Berrangé wrote:
> On Tue, Sep 08, 2020 at 08:24:35PM +0200, Philippe Mathieu-Daudé wrote:
>> Hi Daniel,
>>
>> On 9/8/20 6:54 PM, Daniel P. Berrangé wrote:
>>> Some applications want to pass quite large values for the OEM strings
>>> entries. Rather than having huge strings on the command line, it would
>>> be better to load them from a file, as supported with -fw_cfg.
>>>
>>> This introduces the "valuefile" parameter allowing for:
>>>
>>> $ echo -n "thisthing" > mydata.txt
>>> $ qemu-system-x86_64 \
>>> -smbios type=11,value=something \
>>> -smbios type=11,valuefile=mydata.txt \
>>> -smbios type=11,value=somemore \
>>> ...other args...
>>>
>>> Now in the guest
>>>
>>> $ dmidecide -t 11
>>> Getting SMBIOS data from sysfs.
>>> SMBIOS 2.8 present.
>>>
>>> Handle 0x0E00, DMI type 11, 5 bytes
>>> OEM Strings
>>> String 1: something
>>> String 2: thisthing
>>> String 3: somemore
>>>
>>> Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
>>> ---
>>> hw/smbios/smbios.c | 72 +++++++++++++++++++++++++++++++++++++---------
>>> 1 file changed, 59 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
>>> index 7cc950b41c..8450fad285 100644
>>> --- a/hw/smbios/smbios.c
>>> +++ b/hw/smbios/smbios.c
>>> @@ -110,7 +110,7 @@ static struct {
>>>
>>> static struct {
>>> size_t nvalues;
>>> - const char **values;
>>> + char **values;
>>> } type11;
>>>
>>> static struct {
>>> @@ -314,6 +314,11 @@ static const QemuOptDesc qemu_smbios_type11_opts[] = {
>>> .type = QEMU_OPT_STRING,
>>> .help = "OEM string data",
>>> },
>>> + {
>>> + .name = "path",
>>> + .type = QEMU_OPT_STRING,
>>> + .help = "OEM string data from file",
>>> + },
>>> };
>>>
>>> static const QemuOptDesc qemu_smbios_type17_opts[] = {
>>> @@ -641,6 +646,8 @@ static void smbios_build_type_11_table(void)
>>>
>>> for (i = 0; i < type11.nvalues; i++) {
>>> SMBIOS_TABLE_SET_STR_LIST(11, type11.values[i]);
>>> + g_free(type11.values[i]);
>>> + type11.values[i] = NULL;
>>> }
>>>
>>> SMBIOS_BUILD_TABLE_POST;
>>> @@ -940,9 +947,8 @@ static void save_opt(const char **dest, QemuOpts *opts,
>>> const char *name)
>>>
>>>
>>> struct opt_list {
>>> - const char *name;
>>> size_t *ndest;
>>> - const char ***dest;
>>> + char ***dest;
>>> };
>>>
>>> static int save_opt_one(void *opaque,
>>> @@ -951,23 +957,61 @@ static int save_opt_one(void *opaque,
>>> {
>>> struct opt_list *opt = opaque;
>>>
>>> - if (!g_str_equal(name, opt->name)) {
>>> - return 0;
>>> + if (g_str_equal(name, "path")) {
>>> + g_autoptr(GByteArray) data = g_byte_array_new();
>>> + g_autofree char *buf = g_new(char, 4096);
>>> + ssize_t ret;
>>> + int fd = qemu_open(value, O_RDONLY);
>>
>> While not use g_file_get_contents()?
>
> qemu_open lets mgmt apps pass in pre-opened FDs using /dev/fdset/NN
> syntax.
OK, I was expecting a specific reason from a GLib advocate like you :)
Maybe add a comment on the 'qemu_open()' line? Else one can be tempted
to convert it to g_file_get_contents().
Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
