[PATCH v2 0/7] virsysinfo: Parse OEM strings

v2 of: https://www.redhat.com/archives/libvir-list/2020-June/msg00038.html diff to v1: - cleaned up sysinfotest so that it can use virCommandSetDryRun() - Handle multiline strings (per Dan's suggestion in review of v1) Michal Prívozník (7): virSysinfoReadDMI: Use more g_auto*() virSysinfoReadDMI: Drop needless virFindFileInPath() testSysinfo: Use more g_auto*() sysinfotest: Dissolve sysinfotest_run() in testSysinfo() sysinfotest: Move from custom dmidecode scripts to virCommandSetDryRun() virsysinfo: Drop global @sysinfoDmidecode virsysinfo: Parse OEM strings src/util/virsysinfo.c | 151 +++++++++++++----- src/util/virsysinfopriv.h | 3 +- .../sysinfodata/aarch64-gigabytedmidecode.sh | 3 - tests/sysinfodata/x86dmidecode.sh | 3 - tests/sysinfodata/x86sysinfo.data | 6 + tests/sysinfodata/x86sysinfo.expect | 8 + tests/sysinfotest.c | 144 +++++++++-------- 7 files changed, 203 insertions(+), 115 deletions(-) delete mode 100755 tests/sysinfodata/aarch64-gigabytedmidecode.sh delete mode 100755 tests/sysinfodata/x86dmidecode.sh -- 2.26.2

Virtually every variable defined in the function can be freed automatically when going out of scope. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virsysinfo.c | 36 +++++++++++++----------------------- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 41f4d1cff9..296a2e2cc1 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -1119,10 +1119,10 @@ virSysinfoParseX86Memory(const char *base, virSysinfoDefPtr ret) virSysinfoDefPtr virSysinfoReadDMI(void) { - char *path; - virSysinfoDefPtr ret = NULL; - char *outbuf = NULL; - virCommandPtr cmd; + g_autofree char *path = NULL; + g_auto(virSysinfoDefPtr) ret = NULL; + g_autofree char *outbuf = NULL; + g_autoptr(virCommand) cmd = NULL; path = virFindFileInPath(SYSINFO_SMBIOS_DECODER); if (path == NULL) { @@ -1133,48 +1133,38 @@ virSysinfoReadDMI(void) } cmd = virCommandNewArgList(path, "-q", "-t", "0,1,2,3,4,17", NULL); - VIR_FREE(path); virCommandSetOutputBuffer(cmd, &outbuf); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return NULL; if (VIR_ALLOC(ret) < 0) - goto error; + return NULL; ret->type = VIR_SYSINFO_SMBIOS; if (virSysinfoParseBIOS(outbuf, &ret->bios) < 0) - goto error; + return NULL; if (virSysinfoParseX86System(outbuf, &ret->system) < 0) - goto error; + return NULL; if (virSysinfoParseX86BaseBoard(outbuf, &ret->baseBoard, &ret->nbaseBoard) < 0) - goto error; + return NULL; if (virSysinfoParseX86Chassis(outbuf, &ret->chassis) < 0) - goto error; + return NULL; ret->nprocessor = 0; ret->processor = NULL; if (virSysinfoParseX86Processor(outbuf, ret) < 0) - goto error; + return NULL; ret->nmemory = 0; ret->memory = NULL; if (virSysinfoParseX86Memory(outbuf, ret) < 0) - goto error; + return NULL; - cleanup: - VIR_FREE(outbuf); - virCommandFree(cmd); - - return ret; - - error: - virSysinfoDefFree(ret); - ret = NULL; - goto cleanup; + return g_steal_pointer(&ret); } -- 2.26.2

When trying to decode DMI table, just before constructing virCommand() the decoder is looked for in PATH using virFindFileInPath(). Well, this is not necessary because virCommandRun() will do this too (in virExec()). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virsysinfo.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 296a2e2cc1..0f1210ab37 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -1119,20 +1119,12 @@ virSysinfoParseX86Memory(const char *base, virSysinfoDefPtr ret) virSysinfoDefPtr virSysinfoReadDMI(void) { - g_autofree char *path = NULL; g_auto(virSysinfoDefPtr) ret = NULL; g_autofree char *outbuf = NULL; g_autoptr(virCommand) cmd = NULL; - path = virFindFileInPath(SYSINFO_SMBIOS_DECODER); - if (path == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to find path for %s binary"), - SYSINFO_SMBIOS_DECODER); - return NULL; - } - - cmd = virCommandNewArgList(path, "-q", "-t", "0,1,2,3,4,17", NULL); + cmd = virCommandNewArgList(SYSINFO_SMBIOS_DECODER, + "-q", "-t", "0,1,2,3,4,17", NULL); virCommandSetOutputBuffer(cmd, &outbuf); if (virCommandRun(cmd, NULL) < 0) return NULL; -- 2.26.2

Some variables defined in the function can be freed automatically when going out of scope. This renders @result variable and cleanup label needless. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/sysinfotest.c | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/tests/sysinfotest.c b/tests/sysinfotest.c index a8a0d0869e..4807c5b1ba 100644 --- a/tests/sysinfotest.c +++ b/tests/sysinfotest.c @@ -47,34 +47,24 @@ struct testSysinfoData { static int testSysinfo(const void *data) { - int result = -1; const char *sysfsActualData; - virSysinfoDefPtr ret = NULL; - virBuffer buf = VIR_BUFFER_INITIALIZER; + g_auto(virSysinfoDefPtr) ret = NULL; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; const struct testSysinfoData *testdata = data; virSysinfoSetup(testdata->decoder, testdata->sysinfo, testdata->cpuinfo); if (!testdata->expected || !(ret = testdata->func())) - goto cleanup; + return -1; if (virSysinfoFormat(&buf, ret) < 0) - goto cleanup; + return -1; if (!(sysfsActualData = virBufferCurrentContent(&buf))) - goto cleanup; + return -1; - if (virTestCompareToFile(sysfsActualData, testdata->expected) < 0) - goto cleanup; - - result = 0; - - cleanup: - virSysinfoDefFree(ret); - virBufferFreeAndReset(&buf); - - return result; + return virTestCompareToFile(sysfsActualData, testdata->expected); } static int -- 2.26.2

There is no real need to have two separate functions. They can be merged together which not only saves couple of lines, but prepares the structure of the code for future expansion. See next commits. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/sysinfotest.c | 70 ++++++++++++++------------------------------- 1 file changed, 22 insertions(+), 48 deletions(-) diff --git a/tests/sysinfotest.c b/tests/sysinfotest.c index 4807c5b1ba..558dd60294 100644 --- a/tests/sysinfotest.c +++ b/tests/sysinfotest.c @@ -37,25 +37,33 @@ #define VIR_FROM_THIS VIR_FROM_NONE struct testSysinfoData { + const char *name; /* test name, also base name for result files */ virSysinfoDefPtr (*func)(void); /* sysinfo gathering function */ - char *decoder; /* name of dmi decoder binary/script */ - char *sysinfo; /* name of /proc/sysinfo substitute file */ - char *cpuinfo; /* name of /proc/cpuinfo substitute file */ - char *expected; /* (required) file containing output of virSysinfoFormat */ + const char *decoder; /* name of dmi decoder binary/script */ }; static int testSysinfo(const void *data) { + const struct testSysinfoData *testdata = data; const char *sysfsActualData; g_auto(virSysinfoDefPtr) ret = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - const struct testSysinfoData *testdata = data; + g_autofree char *sysinfo = NULL; + g_autofree char *cpuinfo = NULL; + g_autofree char *expected = NULL; + g_autofree char *decoder = NULL; - virSysinfoSetup(testdata->decoder, testdata->sysinfo, testdata->cpuinfo); + sysinfo = g_strdup_printf("%s/sysinfodata/%ssysinfo.data", abs_srcdir, testdata->name); + cpuinfo = g_strdup_printf("%s/sysinfodata/%scpuinfo.data", abs_srcdir, testdata->name); + expected = g_strdup_printf("%s/sysinfodata/%ssysinfo.expect", abs_srcdir, testdata->name); - if (!testdata->expected || - !(ret = testdata->func())) + if (testdata->decoder) + decoder = g_strdup_printf("%s/%s", abs_srcdir, testdata->decoder); + + virSysinfoSetup(decoder, sysinfo, cpuinfo); + + if (!(ret = testdata->func())) return -1; if (virSysinfoFormat(&buf, ret) < 0) @@ -64,50 +72,16 @@ testSysinfo(const void *data) if (!(sysfsActualData = virBufferCurrentContent(&buf))) return -1; - return virTestCompareToFile(sysfsActualData, testdata->expected); + return virTestCompareToFile(sysfsActualData, expected); } -static int -sysinfotest_run(const char *test, - virSysinfoDefPtr (*func)(void), - const char *decoder, - const char *sysinfo, - const char *cpuinfo, - const char *expected) -{ - struct testSysinfoData testdata = { 0 }; - int ret = EXIT_FAILURE; - - testdata.func = func; - - if (decoder) - testdata.decoder = g_strdup_printf("%s/%s", abs_srcdir, decoder); - if (sysinfo) - testdata.sysinfo = g_strdup_printf("%s/%s", abs_srcdir, sysinfo); - if (cpuinfo) - testdata.cpuinfo = g_strdup_printf("%s/%s", abs_srcdir, cpuinfo); - if (expected) - testdata.expected = g_strdup_printf("%s/%s", abs_srcdir, expected); - - if (virTestRun(test, testSysinfo, &testdata) < 0) - goto error; - - ret = EXIT_SUCCESS; - - error: - VIR_FREE(testdata.decoder); - VIR_FREE(testdata.sysinfo); - VIR_FREE(testdata.cpuinfo); - VIR_FREE(testdata.expected); - return ret; -} #define TEST_FULL(name, func, decoder) \ - if (sysinfotest_run(name " sysinfo", func, decoder, \ - "/sysinfodata/" name "sysinfo.data", \ - "/sysinfodata/" name "cpuinfo.data", \ - "/sysinfodata/" name "sysinfo.expect") != EXIT_SUCCESS) \ - ret = EXIT_FAILURE + do { \ + struct testSysinfoData data = { name, func, decoder }; \ + if (virTestRun(name " sysinfo", testSysinfo, &data) < 0) \ + ret = EXIT_FAILURE; \ + } while (0) #define TEST(name, func) \ -- 2.26.2

Problem with custom dmidecode scripts is that they are hard to modify, especially if we will want them to act differently based on passed arguments. So far, we have two scripts which do no more than 'cat $sysinfo' where $sysinfo is saved dmidecode output. The virCommandSetDryRun() can be used to trick virSysinfoReadDMI() thinking it executed real dmidecode. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virsysinfo.c | 3 +- .../sysinfodata/aarch64-gigabytedmidecode.sh | 3 -- tests/sysinfodata/x86dmidecode.sh | 3 -- tests/sysinfotest.c | 51 +++++++++++++------ 4 files changed, 38 insertions(+), 22 deletions(-) delete mode 100755 tests/sysinfodata/aarch64-gigabytedmidecode.sh delete mode 100755 tests/sysinfodata/x86dmidecode.sh diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 0f1210ab37..0bf80c339b 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -60,7 +60,8 @@ virSysinfoSetup(const char *dmidecode, const char *sysinfo, const char *cpuinfo) { - sysinfoDmidecode = dmidecode; + if (dmidecode) + sysinfoDmidecode = dmidecode; sysinfoSysinfo = sysinfo; sysinfoCpuinfo = cpuinfo; } diff --git a/tests/sysinfodata/aarch64-gigabytedmidecode.sh b/tests/sysinfodata/aarch64-gigabytedmidecode.sh deleted file mode 100755 index 202918103c..0000000000 --- a/tests/sysinfodata/aarch64-gigabytedmidecode.sh +++ /dev/null @@ -1,3 +0,0 @@ -#!/bin/sh -DATAFILE=`dirname $0`/aarch64-gigabytesysinfo.data -cat $DATAFILE diff --git a/tests/sysinfodata/x86dmidecode.sh b/tests/sysinfodata/x86dmidecode.sh deleted file mode 100755 index 28aed61459..0000000000 --- a/tests/sysinfodata/x86dmidecode.sh +++ /dev/null @@ -1,3 +0,0 @@ -#!/bin/sh -DATAFILE=`dirname $0`/x86sysinfo.data -cat $DATAFILE diff --git a/tests/sysinfotest.c b/tests/sysinfotest.c index 558dd60294..f080569730 100644 --- a/tests/sysinfotest.c +++ b/tests/sysinfotest.c @@ -34,14 +34,39 @@ #define LIBVIRT_VIRSYSINFOPRIV_H_ALLOW #include "virsysinfopriv.h" +#define LIBVIRT_VIRCOMMANDPRIV_H_ALLOW +#include "vircommandpriv.h" + #define VIR_FROM_THIS VIR_FROM_NONE struct testSysinfoData { const char *name; /* test name, also base name for result files */ virSysinfoDefPtr (*func)(void); /* sysinfo gathering function */ - const char *decoder; /* name of dmi decoder binary/script */ }; + +static void +testDMIDecodeDryRun(const char *const*args G_GNUC_UNUSED, + const char *const*env G_GNUC_UNUSED, + const char *input G_GNUC_UNUSED, + char **output, + char **error, + int *status, + void *opaque) +{ + const char *sysinfo = opaque; + + if (virFileReadAll(sysinfo, 10 * 1024 * 1024, output) < 0) { + *error = g_strdup(virGetLastErrorMessage()); + *status = EXIT_FAILURE; + return; + } + + *error = g_strdup(""); + *status = 0; +} + + static int testSysinfo(const void *data) { @@ -52,18 +77,19 @@ testSysinfo(const void *data) g_autofree char *sysinfo = NULL; g_autofree char *cpuinfo = NULL; g_autofree char *expected = NULL; - g_autofree char *decoder = NULL; sysinfo = g_strdup_printf("%s/sysinfodata/%ssysinfo.data", abs_srcdir, testdata->name); cpuinfo = g_strdup_printf("%s/sysinfodata/%scpuinfo.data", abs_srcdir, testdata->name); expected = g_strdup_printf("%s/sysinfodata/%ssysinfo.expect", abs_srcdir, testdata->name); - if (testdata->decoder) - decoder = g_strdup_printf("%s/%s", abs_srcdir, testdata->decoder); + virCommandSetDryRun(NULL, testDMIDecodeDryRun, sysinfo); - virSysinfoSetup(decoder, sysinfo, cpuinfo); + virSysinfoSetup(NULL, sysinfo, cpuinfo); - if (!(ret = testdata->func())) + ret = testdata->func(); + virCommandSetDryRun(NULL, NULL, NULL); + + if (!ret) return -1; if (virSysinfoFormat(&buf, ret) < 0) @@ -76,17 +102,13 @@ testSysinfo(const void *data) } -#define TEST_FULL(name, func, decoder) \ +#define TEST(name, func) \ do { \ - struct testSysinfoData data = { name, func, decoder }; \ + struct testSysinfoData data = { name, func }; \ if (virTestRun(name " sysinfo", testSysinfo, &data) < 0) \ ret = EXIT_FAILURE; \ } while (0) - -#define TEST(name, func) \ - TEST_FULL(name, func, NULL) - static int mymain(void) { @@ -95,13 +117,12 @@ mymain(void) TEST("s390", virSysinfoReadS390); TEST("s390-freq", virSysinfoReadS390); TEST("ppc", virSysinfoReadPPC); - TEST_FULL("x86", virSysinfoReadDMI, "/sysinfodata/x86dmidecode.sh"); + TEST("x86", virSysinfoReadDMI); TEST("arm", virSysinfoReadARM); TEST("arm-rpi2", virSysinfoReadARM); TEST("aarch64", virSysinfoReadARM); TEST("aarch64-moonshot", virSysinfoReadARM); - TEST_FULL("aarch64-gigabyte", virSysinfoReadARM, - "/sysinfodata/aarch64-gigabytedmidecode.sh"); + TEST("aarch64-gigabyte", virSysinfoReadARM); return ret; } -- 2.26.2

Since nobody sets custom dmidecode path anymore, we can drop all code that exists only because of that. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virsysinfo.c | 10 ++-------- src/util/virsysinfopriv.h | 3 +-- tests/sysinfotest.c | 2 +- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 0bf80c339b..a26c27e83e 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -45,23 +45,18 @@ VIR_ENUM_IMPL(virSysinfo, "smbios", ); -static const char *sysinfoDmidecode = DMIDECODE; static const char *sysinfoSysinfo = "/proc/sysinfo"; static const char *sysinfoCpuinfo = "/proc/cpuinfo"; -#define SYSINFO_SMBIOS_DECODER sysinfoDmidecode #define SYSINFO sysinfoSysinfo #define CPUINFO sysinfoCpuinfo #define CPUINFO_FILE_LEN (1024*1024) /* 1MB limit for /proc/cpuinfo file */ void -virSysinfoSetup(const char *dmidecode, - const char *sysinfo, +virSysinfoSetup(const char *sysinfo, const char *cpuinfo) { - if (dmidecode) - sysinfoDmidecode = dmidecode; sysinfoSysinfo = sysinfo; sysinfoCpuinfo = cpuinfo; } @@ -1124,8 +1119,7 @@ virSysinfoReadDMI(void) g_autofree char *outbuf = NULL; g_autoptr(virCommand) cmd = NULL; - cmd = virCommandNewArgList(SYSINFO_SMBIOS_DECODER, - "-q", "-t", "0,1,2,3,4,17", NULL); + cmd = virCommandNewArgList(DMIDECODE, "-q", "-t", "0,1,2,3,4,17", NULL); virCommandSetOutputBuffer(cmd, &outbuf); if (virCommandRun(cmd, NULL) < 0) return NULL; diff --git a/src/util/virsysinfopriv.h b/src/util/virsysinfopriv.h index cdcb021309..d6c558a32d 100644 --- a/src/util/virsysinfopriv.h +++ b/src/util/virsysinfopriv.h @@ -24,8 +24,7 @@ #pragma once void -virSysinfoSetup(const char *dmidecode, - const char *sysinfo, +virSysinfoSetup(const char *sysinfo, const char *cpuinfo); virSysinfoDefPtr diff --git a/tests/sysinfotest.c b/tests/sysinfotest.c index f080569730..10d24b823a 100644 --- a/tests/sysinfotest.c +++ b/tests/sysinfotest.c @@ -84,7 +84,7 @@ testSysinfo(const void *data) virCommandSetDryRun(NULL, testDMIDecodeDryRun, sysinfo); - virSysinfoSetup(NULL, sysinfo, cpuinfo); + virSysinfoSetup(sysinfo, cpuinfo); ret = testdata->func(); virCommandSetDryRun(NULL, NULL, NULL); -- 2.26.2

Setting OEM strings for a domain was introduced in v4.1.0-rc1~315. However, any application that wanted to use them (e.g. to point to an URL where a config file is stored) had to 'dmidecode -u --oem-string N' (where N is index of the string). Well, we can expose them under our <sysinfo/> XML and if the domain is running Libvirt inside it can be obtained using virConnectGetSysinfo() API. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virsysinfo.c | 102 +++++++++++++++++++++++++++- tests/sysinfodata/x86sysinfo.data | 6 ++ tests/sysinfodata/x86sysinfo.expect | 8 +++ tests/sysinfotest.c | 27 ++++++-- 4 files changed, 138 insertions(+), 5 deletions(-) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index a26c27e83e..09e32df6a9 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -915,6 +915,103 @@ virSysinfoParseX86Chassis(const char *base, } +static int +virSysinfoDMIDecodeOEMString(size_t i, + char **str) +{ + g_autofree char *err = NULL; + g_autoptr(virCommand) cmd = virCommandNewArgList(DMIDECODE, "--dump", + "--oem-string", NULL); + virCommandAddArgFormat(cmd, "%zu", i); + virCommandSetOutputBuffer(cmd, str); + virCommandSetErrorBuffer(cmd, &err); + + if (virCommandRun(cmd, NULL) < 0) + return -1; + + /* Unfortunately, dmidecode returns 0 even if OEM String index is out + * of bounds, but it prints an error message in that case. Check stderr + * and return success/failure accordingly. */ + + if (err && *err != '\0') + return -1; + + return 0; +} + + +static int +virSysinfoParseOEMStrings(const char *base, + virSysinfoOEMStringsDefPtr *stringsRet) +{ + virSysinfoOEMStringsDefPtr strings = NULL; + size_t i = 1; + int ret = -1; + const char *cur; + + if (!(cur = strstr(base, "OEM Strings"))) + return 0; + + if (VIR_ALLOC(strings) < 0) + return -1; + + while ((cur = strstr(cur, "String "))) { + char *eol; + + cur += 7; + + if (!(eol = strchr(cur, '\n'))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed output of dmidecode")); + goto cleanup; + } + + while (g_ascii_isdigit(*cur)) + cur++; + + if (*cur != ':') { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed output of dmidecode")); + goto cleanup; + } + + cur += 2; + + virSkipSpacesBackwards(cur, &eol); + if (!eol) + continue; + + if (VIR_EXPAND_N(strings->values, strings->nvalues, 1) < 0) + goto cleanup; + + /* If OEM String contains newline, dmidecode escapes it as a dot. + * If this is the case then run dmidecode again to get raw string. + * Unfortunately, we can't dinstinguish betwen dot an new line at + * this level. */ + if (memchr(cur, '.', eol - cur)) { + char *str; + + if (virSysinfoDMIDecodeOEMString(i, &str) < 0) + goto cleanup; + + strings->values[strings->nvalues - 1] = g_steal_pointer(&str); + } else { + strings->values[strings->nvalues - 1] = g_strndup(cur, eol - cur); + } + + i++; + cur = eol; + } + + *stringsRet = g_steal_pointer(&strings); + ret = 0; + + cleanup: + virSysinfoOEMStringsDefFree(strings); + return ret; +} + + static int virSysinfoParseX86Processor(const char *base, virSysinfoDefPtr ret) { @@ -1119,7 +1216,7 @@ virSysinfoReadDMI(void) g_autofree char *outbuf = NULL; g_autoptr(virCommand) cmd = NULL; - cmd = virCommandNewArgList(DMIDECODE, "-q", "-t", "0,1,2,3,4,17", NULL); + cmd = virCommandNewArgList(DMIDECODE, "-q", "-t", "0,1,2,3,4,11,17", NULL); virCommandSetOutputBuffer(cmd, &outbuf); if (virCommandRun(cmd, NULL) < 0) return NULL; @@ -1141,6 +1238,9 @@ virSysinfoReadDMI(void) if (virSysinfoParseX86Chassis(outbuf, &ret->chassis) < 0) return NULL; + if (virSysinfoParseOEMStrings(outbuf, &ret->oemStrings) < 0) + return NULL; + ret->nprocessor = 0; ret->processor = NULL; if (virSysinfoParseX86Processor(outbuf, ret) < 0) diff --git a/tests/sysinfodata/x86sysinfo.data b/tests/sysinfodata/x86sysinfo.data index 426261041d..3f0b654e4b 100644 --- a/tests/sysinfodata/x86sysinfo.data +++ b/tests/sysinfodata/x86sysinfo.data @@ -81,3 +81,9 @@ Memory Device Serial Number: 29057112 Asset Tag: 0839 Part Number: IMSH2GS13A1F1C-10F + +OEM Strings + String 1: Hello + String 2: World + String 3: Ha ha ha try parsing\n. String 3: this correctly. String 4:then + String 4: This is, more tricky value=escaped diff --git a/tests/sysinfodata/x86sysinfo.expect b/tests/sysinfodata/x86sysinfo.expect index fcdd790cbd..05add8f031 100644 --- a/tests/sysinfodata/x86sysinfo.expect +++ b/tests/sysinfodata/x86sysinfo.expect @@ -50,4 +50,12 @@ <entry name='serial_number'>29057112</entry> <entry name='part_number'>IMSH2GS13A1F1C-10F</entry> </memory_device> + <oemStrings> + <entry>Hello</entry> + <entry>World</entry> + <entry>Ha ha ha try parsing\n + String 3: this correctly + String 4:then</entry> + <entry>This is, more tricky value=escaped</entry> + </oemStrings> </sysinfo> diff --git a/tests/sysinfotest.c b/tests/sysinfotest.c index 10d24b823a..3b418955d0 100644 --- a/tests/sysinfotest.c +++ b/tests/sysinfotest.c @@ -56,10 +56,29 @@ testDMIDecodeDryRun(const char *const*args G_GNUC_UNUSED, { const char *sysinfo = opaque; - if (virFileReadAll(sysinfo, 10 * 1024 * 1024, output) < 0) { - *error = g_strdup(virGetLastErrorMessage()); - *status = EXIT_FAILURE; - return; + if (STREQ_NULLABLE(args[1], "--dump") && + STREQ_NULLABLE(args[2], "--oem-string")) { + if (!args[3]) { + *error = g_strdup("dmidecode: option '--oem-string' requires an argument"); + *status = EXIT_FAILURE; + return; + } + + if (STREQ(args[3], "3")) { + *output = g_strdup("Ha ha ha try parsing\\n\n" + " String 3: this correctly\n" + " String 4:then"); + } else { + *error = g_strdup_printf("No OEM string number %s", args[3]); + *status = EXIT_FAILURE; + return; + } + } else { + if (virFileReadAll(sysinfo, 10 * 1024 * 1024, output) < 0) { + *error = g_strdup(virGetLastErrorMessage()); + *status = EXIT_FAILURE; + return; + } } *error = g_strdup(""); -- 2.26.2

On Mon, Jun 08, 2020 at 05:06:14PM +0200, Michal Privoznik wrote:
v2 of:
https://www.redhat.com/archives/libvir-list/2020-June/msg00038.html
diff to v1: - cleaned up sysinfotest so that it can use virCommandSetDryRun() - Handle multiline strings (per Dan's suggestion in review of v1)
Michal Prívozník (7): virSysinfoReadDMI: Use more g_auto*() virSysinfoReadDMI: Drop needless virFindFileInPath() testSysinfo: Use more g_auto*() sysinfotest: Dissolve sysinfotest_run() in testSysinfo() sysinfotest: Move from custom dmidecode scripts to virCommandSetDryRun() virsysinfo: Drop global @sysinfoDmidecode virsysinfo: Parse OEM strings
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> 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 :|
participants (2)
-
Daniel P. Berrangé
-
Michal Privoznik