[libvirt] [PATCH v2] virSysinfo: Introduce SMBIOS type 2 support
https://bugzilla.redhat.com/show_bug.cgi?id=1220527 This type of information defines attributes of a system baseboard. With one caveat: in qemu they call it family, while in the specification it's referred to as type. I'm sticking with the latter. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- diff to v1: -I've dropped the 'family' attribute. It's not implemented in qemu yet. We can add it later. docs/formatdomain.html.in | 37 ++++- docs/schemas/domaincommon.rng | 23 +++ src/conf/domain_conf.c | 63 ++++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 54 +++++++ src/util/virsysinfo.c | 170 ++++++++++++++++++++- src/util/virsysinfo.h | 16 ++ .../qemuxml2argv-smbios-multiple-type2.xml | 58 +++++++ tests/qemuxml2argvdata/qemuxml2argv-smbios.args | 2 + tests/qemuxml2argvdata/qemuxml2argv-smbios.xml | 8 + tests/qemuxml2xmltest.c | 1 + 11 files changed, 427 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-smbios-multiple-type2.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0478cb2..977660e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -375,6 +375,12 @@ <entry name='product'>Virt-Manager</entry> <entry name='version'>0.9.4</entry> </system> + <baseBoard> + <entry name='manufacturer'>LENOVO</entry> + <entry name='product'>20BE0061MC</entry> + <entry name='version'>0B98401 Pro</entry> + <entry name='serial'>W1KS427111E</entry> + </baseBoard> </sysinfo> ...</pre> @@ -435,11 +441,32 @@ <dt><code>family</code></dt> <dd>Identify the family a particular computer belongs to.</dd> </dl> - NB: Incorrectly supplied entries in either the <code>bios</code> - or <code>system</code> blocks will be ignored without error. - Other than <code>uuid</code> validation and <code>date</code> - format checking, all values are passed as strings to the - hypervisor driver. + </dd> + <dt><code>baseBoard</code></dt> + <dd> + This is block 2 of SMBIOS. This element can be repeated multiple + times, to describe all the base boards. However, not all + hypervisors support the repetition necessarily. The element can + have the following children: + <dl> + <dt><code>manufacturer</code></dt> + <dd>Manufacturer of BIOS</dd> + <dt><code>product</code></dt> + <dd>Product Name</dd> + <dt><code>version</code></dt> + <dd>Version of the product</dd> + <dt><code>serial</code></dt> + <dd>Serial number</dd> + <dt><code>asset</code></dt> + <dd>Asset tag</dd> + <dt><code>location</code></dt> + <dd>Location in chassis</dd> + </dl> + NB: Incorrectly supplied entries for the + <code>bios</code>, <code>system</code> or <code>baseBoard</code> + blocks will be ignored without error. Other than <code>uuid</code> + validation and <code>date</code> format checking, all values are + passed as strings to the hypervisor driver. </dd> </dl> </dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index e38b927..32d28cd 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4248,6 +4248,18 @@ </oneOrMore> </element> </optional> + <zeroOrMore> + <element name="baseBoard"> + <oneOrMore> + <element name="entry"> + <attribute name="name"> + <ref name="sysinfo-baseBoard-name"/> + </attribute> + <ref name="sysinfo-value"/> + </element> + </oneOrMore> + </element> + </zeroOrMore> </interleave> </element> </define> @@ -4273,6 +4285,17 @@ </choice> </define> + <define name="sysinfo-baseBoard-name"> + <choice> + <value>manufacturer</value> + <value>product</value> + <value>version</value> + <value>serial</value> + <value>asset</value> + <value>location</value> + </choice> + </define> + <define name="sysinfo-value"> <data type="string"> <param name='pattern'>[a-zA-Z0-9/\-_\. \(\)]+</param> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bf7eeb2..c2174d9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11227,6 +11227,65 @@ virSysinfoSystemParseXML(xmlNodePtr node, return ret; } +static int +virSysinfoBaseBoardParseXML(xmlXPathContextPtr ctxt, + virSysinfoBaseBoardDefPtr *baseBoard, + size_t *nbaseBoard) +{ + int ret = -1; + virSysinfoBaseBoardDefPtr boards = NULL; + size_t i, nboards = 0; + char *board_type = NULL; + xmlNodePtr *nodes = NULL, oldnode = ctxt->node; + int n; + + if ((n = virXPathNodeSet("./baseBoard", ctxt, &nodes)) < 0) + return ret; + + if (n && VIR_ALLOC_N(boards, n) < 0) + goto cleanup; + + for (i = 0; i < n; i++) { + virSysinfoBaseBoardDefPtr def = boards + nboards; + + ctxt->node = nodes[i]; + + def->manufacturer = + virXPathString("string(entry[@name='manufacturer'])", ctxt); + def->product = + virXPathString("string(entry[@name='product'])", ctxt); + def->version = + virXPathString("string(entry[@name='version'])", ctxt); + def->serial = + virXPathString("string(entry[@name='serial'])", ctxt); + def->asset = + virXPathString("string(entry[@name='asset'])", ctxt); + def->location = + virXPathString("string(entry[@name='location'])", ctxt); + + if (!def->manufacturer && !def->product && !def->version && + !def->serial && !def->asset && !def->location) { + /* nada */ + } else { + nboards++; + } + } + + *baseBoard = boards; + *nbaseBoard = nboards; + boards = NULL; + nboards = 0; + ret = 0; + cleanup: + while (nboards--) + virSysinfoBaseBoardDefClear(&boards[nboards]); + VIR_FREE(boards); + VIR_FREE(board_type); + VIR_FREE(nodes); + ctxt->node = oldnode; + return ret; +} + static virSysinfoDefPtr virSysinfoParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -11281,6 +11340,10 @@ virSysinfoParseXML(xmlNodePtr node, ctxt->node = oldnode; } + /* Extract system base board metadata */ + if (virSysinfoBaseBoardParseXML(ctxt, &def->baseBoard, &def->nbaseBoard) < 0) + goto error; + cleanup: VIR_FREE(type); return def; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dc8a52d..2348f14 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2182,6 +2182,7 @@ virVasprintfInternal; # util/virsysinfo.h +virSysinfoBaseBoardDefClear; virSysinfoBIOSDefFree; virSysinfoDefFree; virSysinfoFormat; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c34fe8a..03f8e66 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6779,6 +6779,45 @@ static char *qemuBuildSmbiosSystemStr(virSysinfoSystemDefPtr def, return NULL; } +static char *qemuBuildSmbiosBaseBoardStr(virSysinfoBaseBoardDefPtr def) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (!def) + return NULL; + + virBufferAddLit(&buf, "type=2"); + + /* 2:Manufacturer */ + if (def->manufacturer) + virBufferAsprintf(&buf, ",manufacturer=%s", + def->manufacturer); + /* 2:Product Name */ + if (def->product) + virBufferAsprintf(&buf, ",product=%s", def->product); + /* 2:Version */ + if (def->version) + virBufferAsprintf(&buf, ",version=%s", def->version); + /* 2:Serial Number */ + if (def->serial) + virBufferAsprintf(&buf, ",serial=%s", def->serial); + /* 2:Asset Tag */ + if (def->asset) + virBufferAsprintf(&buf, ",asset=%s", def->asset); + /* 2:Location */ + if (def->location) + virBufferAsprintf(&buf, ",location=%s", def->location); + + if (virBufferCheckError(&buf) < 0) + goto error; + + return virBufferContentAndReset(&buf); + + error: + virBufferFreeAndReset(&buf); + return NULL; +} + static char * qemuBuildClockArgStr(virDomainClockDefPtr def) { @@ -9057,6 +9096,21 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); VIR_FREE(smbioscmd); } + + if (source->nbaseBoard > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("qemu does not support more than " + "one entry to Type 2 in SMBIOS table")); + goto error; + } + + for (i = 0; i < source->nbaseBoard; i++) { + if (!(smbioscmd = qemuBuildSmbiosBaseBoardStr(source->baseBoard + i))) + goto error; + + virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); + VIR_FREE(smbioscmd); + } } } diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 692c921..a4d31c9 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -91,6 +91,18 @@ void virSysinfoSystemDefFree(virSysinfoSystemDefPtr def) VIR_FREE(def); } +void virSysinfoBaseBoardDefClear(virSysinfoBaseBoardDefPtr def) +{ + if (def == NULL) + return; + + VIR_FREE(def->manufacturer); + VIR_FREE(def->product); + VIR_FREE(def->version); + VIR_FREE(def->serial); + VIR_FREE(def->asset); + VIR_FREE(def->location); +} /** * virSysinfoDefFree: @@ -109,6 +121,10 @@ void virSysinfoDefFree(virSysinfoDefPtr def) virSysinfoBIOSDefFree(def->bios); virSysinfoSystemDefFree(def->system); + for (i = 0; i < def->nbaseBoard; i++) + virSysinfoBaseBoardDefClear(def->baseBoard + i); + VIR_FREE(def->baseBoard); + for (i = 0; i < def->nprocessor; i++) { VIR_FREE(def->processor[i].processor_socket_destination); VIR_FREE(def->processor[i].processor_type); @@ -717,6 +733,84 @@ virSysinfoParseSystem(const char *base, virSysinfoSystemDefPtr *system) } static int +virSysinfoParseBaseBoard(const char *base, + virSysinfoBaseBoardDefPtr *baseBoard, + size_t *nbaseBoard) +{ + int ret = -1; + const char *cur, *eol = NULL; + virSysinfoBaseBoardDefPtr boards = NULL; + size_t nboards = 0; + char *board_type = NULL; + + while (base && (cur = strstr(base, "Base Board Information"))) { + virSysinfoBaseBoardDefPtr def; + + if (VIR_EXPAND_N(boards, nboards, 1) < 0) + goto cleanup; + + def = &boards[nboards - 1]; + + base = cur + 22; + if ((cur = strstr(base, "Manufacturer: ")) != NULL) { + cur += 14; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->manufacturer, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Product Name: ")) != NULL) { + cur += 14; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->product, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Version: ")) != NULL) { + cur += 9; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Serial Number: ")) != NULL) { + cur += 15; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->serial, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Asset Tag: ")) != NULL) { + cur += 11; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->asset, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Location In Chassis: ")) != NULL) { + cur += 21; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->location, cur, eol - cur) < 0) + goto cleanup; + } + + if (!def->manufacturer && !def->product && !def->version && + !def->serial && !def->asset && !def->location) + nboards--; + } + + /* This is safe, as we can be only shrinking the memory */ + ignore_value(VIR_REALLOC_N(boards, nboards)); + + *baseBoard = boards; + *nbaseBoard = nboards; + boards = NULL; + nboards = 0; + ret = 0; + cleanup: + while (nboards--) + virSysinfoBaseBoardDefClear(&boards[nboards]); + VIR_FREE(boards); + VIR_FREE(board_type); + return ret; +} + +static int virSysinfoParseProcessor(const char *base, virSysinfoDefPtr ret) { const char *cur, *tmp_base; @@ -938,7 +1032,7 @@ virSysinfoRead(void) return NULL; } - cmd = virCommandNewArgList(path, "-q", "-t", "0,1,4,17", NULL); + cmd = virCommandNewArgList(path, "-q", "-t", "0,1,2,4,17", NULL); VIR_FREE(path); virCommandSetOutputBuffer(cmd, &outbuf); if (virCommandRun(cmd, NULL) < 0) @@ -955,6 +1049,9 @@ virSysinfoRead(void) if (virSysinfoParseSystem(outbuf, &ret->system) < 0) goto error; + if (virSysinfoParseBaseBoard(outbuf, &ret->baseBoard, &ret->nbaseBoard) < 0) + goto error; + ret->nprocessor = 0; ret->processor = NULL; if (virSysinfoParseProcessor(outbuf, ret) < 0) @@ -1025,6 +1122,36 @@ virSysinfoSystemFormat(virBufferPtr buf, virSysinfoSystemDefPtr def) } static void +virSysinfoBaseBoardFormat(virBufferPtr buf, + virSysinfoBaseBoardDefPtr baseBoard, + size_t nbaseBoard) +{ + virSysinfoBaseBoardDefPtr def; + size_t i; + + for (i = 0; i < nbaseBoard; i++) { + def = baseBoard + i; + + virBufferAddLit(buf, "<baseBoard>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<entry name='manufacturer'>%s</entry>\n", + def->manufacturer); + virBufferEscapeString(buf, "<entry name='product'>%s</entry>\n", + def->product); + virBufferEscapeString(buf, "<entry name='version'>%s</entry>\n", + def->version); + virBufferEscapeString(buf, "<entry name='serial'>%s</entry>\n", + def->serial); + virBufferEscapeString(buf, "<entry name='asset'>%s</entry>\n", + def->asset); + virBufferEscapeString(buf, "<entry name='location'>%s</entry>\n", + def->location); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</baseBoard>\n"); + } +} + +static void virSysinfoProcessorFormat(virBufferPtr buf, virSysinfoDefPtr def) { size_t i; @@ -1157,6 +1284,7 @@ virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def) virSysinfoBIOSFormat(&childrenBuf, def->bios); virSysinfoSystemFormat(&childrenBuf, def->system); + virSysinfoBaseBoardFormat(&childrenBuf, def->baseBoard, def->nbaseBoard); virSysinfoProcessorFormat(&childrenBuf, def); virSysinfoMemoryFormat(&childrenBuf, def); @@ -1241,12 +1369,40 @@ virSysinfoSystemIsEqual(virSysinfoSystemDefPtr src, return identical; } +static bool +virSysinfoBaseBoardIsEqual(virSysinfoBaseBoardDefPtr src, + virSysinfoBaseBoardDefPtr dst) +{ + bool identical = false; + + if (!src && !dst) + return true; + + if ((src && !dst) || (!src && dst)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Target base board does not match source")); + goto cleanup; + } + + CHECK_FIELD(manufacturer, "base board vendor"); + CHECK_FIELD(product, "base board product"); + CHECK_FIELD(version, "base board version"); + CHECK_FIELD(serial, "base board serial"); + CHECK_FIELD(asset, "base board asset"); + CHECK_FIELD(location, "base board location"); + + identical = true; + cleanup: + return identical; +} + #undef CHECK_FIELD bool virSysinfoIsEqual(virSysinfoDefPtr src, virSysinfoDefPtr dst) { bool identical = false; + size_t i; if (!src && !dst) return true; @@ -1271,6 +1427,18 @@ bool virSysinfoIsEqual(virSysinfoDefPtr src, if (!virSysinfoSystemIsEqual(src->system, dst->system)) goto cleanup; + if (src->nbaseBoard != dst->nbaseBoard) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target sysinfo base board count '%zu' does not match source '%zu'"), + dst->nbaseBoard, src->nbaseBoard); + goto cleanup; + } + + for (i = 0; i < src->nbaseBoard; i++) + if (!virSysinfoBaseBoardIsEqual(src->baseBoard + i, + dst->baseBoard + i)) + goto cleanup; + identical = true; cleanup: diff --git a/src/util/virsysinfo.h b/src/util/virsysinfo.h index c8cc1e8..1e51d2c 100644 --- a/src/util/virsysinfo.h +++ b/src/util/virsysinfo.h @@ -86,6 +86,18 @@ struct _virSysinfoSystemDef { char *family; }; +typedef struct _virSysinfoBaseBoardDef virSysinfoBaseBoardDef; +typedef virSysinfoBaseBoardDef *virSysinfoBaseBoardDefPtr; +struct _virSysinfoBaseBoardDef { + char *manufacturer; + char *product; + char *version; + char *serial; + char *asset; + char *location; + /* XXX board type */ +}; + typedef struct _virSysinfoDef virSysinfoDef; typedef virSysinfoDef *virSysinfoDefPtr; struct _virSysinfoDef { @@ -94,6 +106,9 @@ struct _virSysinfoDef { virSysinfoBIOSDefPtr bios; virSysinfoSystemDefPtr system; + size_t nbaseBoard; + virSysinfoBaseBoardDefPtr baseBoard; + size_t nprocessor; virSysinfoProcessorDefPtr processor; @@ -105,6 +120,7 @@ virSysinfoDefPtr virSysinfoRead(void); void virSysinfoBIOSDefFree(virSysinfoBIOSDefPtr def); void virSysinfoSystemDefFree(virSysinfoSystemDefPtr def); +void virSysinfoBaseBoardDefClear(virSysinfoBaseBoardDefPtr def); void virSysinfoDefFree(virSysinfoDefPtr def); int virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios-multiple-type2.xml b/tests/qemuxml2argvdata/qemuxml2argv-smbios-multiple-type2.xml new file mode 100644 index 0000000..1f6aec1 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios-multiple-type2.xml @@ -0,0 +1,58 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <sysinfo type='smbios'> + <bios> + <entry name='vendor'>LENOVO</entry> + <entry name='version'>6FET82WW (3.12 )</entry> + </bios> + <system> + <entry name='manufacturer'>Fedora</entry> + <entry name='product'>Virt-Manager</entry> + <entry name='version'>0.8.2-3.fc14</entry> + <entry name='serial'>32dfcb37-5af1-552b-357c-be8c3aa38310</entry> + <entry name='uuid'>c7a5fdbd-edaf-9455-926a-d65c16db1809</entry> + <entry name='sku'>1234567890</entry> + <entry name='family'>Red Hat</entry> + </system> + <baseBoard> + <entry name='manufacturer'>Hewlett-Packard</entry> + <entry name='product'>0B4Ch</entry> + <entry name='version'>D</entry> + <entry name='serial'>CZC1065993</entry> + <entry name='asset'>CZC1065993</entry> + <entry name='location'>Upside down</entry> + </baseBoard> + <baseBoard> + <entry name='manufacturer'>Lenovo</entry> + <entry name='product'>20BE0061MC</entry> + <entry name='version'>0B98401 Pro</entry> + <entry name='serial'>W1KS427111E</entry> + <entry name='location'>Not Available</entry> + </baseBoard> + </sysinfo> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + <smbios mode='sysinfo'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args index e939aca..209cf20 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args @@ -4,5 +4,7 @@ pc -m 214 -smp 1 -smbios 'type=0,vendor=LENOVO,version=6FET82WW (3.12 )' \ -smbios 'type=1,manufacturer=Fedora,product=Virt-Manager,version=0.8.2-3.fc14,\ serial=32dfcb37-5af1-552b-357c-be8c3aa38310,\ uuid=c7a5fdbd-edaf-9455-926a-d65c16db1809,sku=1234567890,family=Red Hat' \ +-smbios 'type=2,manufacturer=Hewlett-Packard,product=0B4Ch,version=D,\ +serial=CZC1065993,asset=CZC1065993,location=Upside down' \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml b/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml index a2caeea..30888ae 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml @@ -18,6 +18,14 @@ <entry name='sku'>1234567890</entry> <entry name='family'>Red Hat</entry> </system> + <baseBoard> + <entry name='manufacturer'>Hewlett-Packard</entry> + <entry name='product'>0B4Ch</entry> + <entry name='version'>D</entry> + <entry name='serial'>CZC1065993</entry> + <entry name='asset'>CZC1065993</entry> + <entry name='location'>Upside down</entry> + </baseBoard> </sysinfo> <os> <type arch='i686' machine='pc'>hvm</type> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 44b388c..3287ea3 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -614,6 +614,7 @@ mymain(void) DO_TEST_DIFFERENT("tap-vhost-incorrect"); DO_TEST("shmem"); DO_TEST("smbios"); + DO_TEST("smbios-multiple-type2"); DO_TEST("aarch64-aavmf-virtio-mmio"); DO_TEST("memory-hotplug"); -- 2.3.6
On 06/12/2015 06:02 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1220527
This type of information defines attributes of a system baseboard. With one caveat: in qemu they call it family, while in the specification it's referred to as type. I'm sticking with the latter.
Perhaps should update this since 'family' and 'type' aren't processed by libvirt.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
diff to v1: -I've dropped the 'family' attribute. It's not implemented in qemu yet. We can add it later.
docs/formatdomain.html.in | 37 ++++- docs/schemas/domaincommon.rng | 23 +++ src/conf/domain_conf.c | 63 ++++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 54 +++++++ src/util/virsysinfo.c | 170 ++++++++++++++++++++- src/util/virsysinfo.h | 16 ++ .../qemuxml2argv-smbios-multiple-type2.xml | 58 +++++++ tests/qemuxml2argvdata/qemuxml2argv-smbios.args | 2 + tests/qemuxml2argvdata/qemuxml2argv-smbios.xml | 8 + tests/qemuxml2xmltest.c | 1 + 11 files changed, 427 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-smbios-multiple-type2.xml
There's a couple issues pointed out below, fixable without need for a v3 I believe... ACK with the adjustments John
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0478cb2..977660e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -375,6 +375,12 @@ <entry name='product'>Virt-Manager</entry> <entry name='version'>0.9.4</entry> </system> + <baseBoard> + <entry name='manufacturer'>LENOVO</entry> + <entry name='product'>20BE0061MC</entry> + <entry name='version'>0B98401 Pro</entry> + <entry name='serial'>W1KS427111E</entry> + </baseBoard> </sysinfo> ...</pre>
@@ -435,11 +441,32 @@ <dt><code>family</code></dt> <dd>Identify the family a particular computer belongs to.</dd> </dl> - NB: Incorrectly supplied entries in either the <code>bios</code> - or <code>system</code> blocks will be ignored without error. - Other than <code>uuid</code> validation and <code>date</code> - format checking, all values are passed as strings to the - hypervisor driver. + </dd> + <dt><code>baseBoard</code></dt> + <dd> + This is block 2 of SMBIOS. This element can be repeated multiple + times, to describe all the base boards. However, not all
s/times,/times s/boards. However,/boards; however,
+ hypervisors support the repetition necessarily. The element can
s/support the repetition necessarily/necessarily support the repitition or s/support the repetition necessarily./support multiple base boards./
+ have the following children: + <dl> + <dt><code>manufacturer</code></dt> + <dd>Manufacturer of BIOS</dd> + <dt><code>product</code></dt> + <dd>Product Name</dd> + <dt><code>version</code></dt> + <dd>Version of the product</dd> + <dt><code>serial</code></dt> + <dd>Serial number</dd> + <dt><code>asset</code></dt> + <dd>Asset tag</dd> + <dt><code>location</code></dt> + <dd>Location in chassis</dd> + </dl> + NB: Incorrectly supplied entries for the + <code>bios</code>, <code>system</code> or <code>baseBoard</code> + blocks will be ignored without error. Other than <code>uuid</code> + validation and <code>date</code> format checking, all values are + passed as strings to the hypervisor driver. </dd> </dl> </dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index e38b927..32d28cd 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4248,6 +4248,18 @@ </oneOrMore> </element> </optional> + <zeroOrMore> + <element name="baseBoard"> + <oneOrMore> + <element name="entry"> + <attribute name="name"> + <ref name="sysinfo-baseBoard-name"/> + </attribute> + <ref name="sysinfo-value"/> + </element> + </oneOrMore> + </element> + </zeroOrMore> </interleave> </element> </define> @@ -4273,6 +4285,17 @@ </choice> </define>
+ <define name="sysinfo-baseBoard-name"> + <choice> + <value>manufacturer</value> + <value>product</value> + <value>version</value> + <value>serial</value> + <value>asset</value> + <value>location</value> + </choice> + </define> + <define name="sysinfo-value"> <data type="string"> <param name='pattern'>[a-zA-Z0-9/\-_\. \(\)]+</param> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bf7eeb2..c2174d9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11227,6 +11227,65 @@ virSysinfoSystemParseXML(xmlNodePtr node, return ret; }
+static int +virSysinfoBaseBoardParseXML(xmlXPathContextPtr ctxt, + virSysinfoBaseBoardDefPtr *baseBoard, + size_t *nbaseBoard) +{ + int ret = -1; + virSysinfoBaseBoardDefPtr boards = NULL; + size_t i, nboards = 0; + char *board_type = NULL;
Unused
+ xmlNodePtr *nodes = NULL, oldnode = ctxt->node; + int n; + + if ((n = virXPathNodeSet("./baseBoard", ctxt, &nodes)) < 0) + return ret; + + if (n && VIR_ALLOC_N(boards, n) < 0) + goto cleanup; + + for (i = 0; i < n; i++) { + virSysinfoBaseBoardDefPtr def = boards + nboards; + + ctxt->node = nodes[i]; + + def->manufacturer = + virXPathString("string(entry[@name='manufacturer'])", ctxt); + def->product = + virXPathString("string(entry[@name='product'])", ctxt); + def->version = + virXPathString("string(entry[@name='version'])", ctxt); + def->serial = + virXPathString("string(entry[@name='serial'])", ctxt); + def->asset = + virXPathString("string(entry[@name='asset'])", ctxt); + def->location = + virXPathString("string(entry[@name='location'])", ctxt); + + if (!def->manufacturer && !def->product && !def->version && + !def->serial && !def->asset && !def->location) { + /* nada */ + } else { + nboards++; + } + } + + *baseBoard = boards; + *nbaseBoard = nboards; + boards = NULL; + nboards = 0; + ret = 0; + cleanup: + while (nboards--) + virSysinfoBaseBoardDefClear(&boards[nboards]);
Coverity notes nboards can never be anything other than zero when we get here. IOW we can never jump out of that for loop where nboards gets incremented unlike virSysinfoParseBaseBoard where the VIR_EXPAND_N failure...
+ VIR_FREE(boards); + VIR_FREE(board_type);
Unused
+ VIR_FREE(nodes); + ctxt->node = oldnode; + return ret; +} + static virSysinfoDefPtr virSysinfoParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -11281,6 +11340,10 @@ virSysinfoParseXML(xmlNodePtr node, ctxt->node = oldnode; }
+ /* Extract system base board metadata */ + if (virSysinfoBaseBoardParseXML(ctxt, &def->baseBoard, &def->nbaseBoard) < 0) + goto error; + cleanup: VIR_FREE(type); return def; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dc8a52d..2348f14 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2182,6 +2182,7 @@ virVasprintfInternal;
# util/virsysinfo.h +virSysinfoBaseBoardDefClear;
Probably won't be necessary now that domain_conf.c doesn't need it...
virSysinfoBIOSDefFree; virSysinfoDefFree; virSysinfoFormat; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c34fe8a..03f8e66 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6779,6 +6779,45 @@ static char *qemuBuildSmbiosSystemStr(virSysinfoSystemDefPtr def, return NULL; }
+static char *qemuBuildSmbiosBaseBoardStr(virSysinfoBaseBoardDefPtr def) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (!def) + return NULL; + + virBufferAddLit(&buf, "type=2"); + + /* 2:Manufacturer */ + if (def->manufacturer) + virBufferAsprintf(&buf, ",manufacturer=%s", + def->manufacturer); + /* 2:Product Name */ + if (def->product) + virBufferAsprintf(&buf, ",product=%s", def->product); + /* 2:Version */ + if (def->version) + virBufferAsprintf(&buf, ",version=%s", def->version); + /* 2:Serial Number */ + if (def->serial) + virBufferAsprintf(&buf, ",serial=%s", def->serial); + /* 2:Asset Tag */ + if (def->asset) + virBufferAsprintf(&buf, ",asset=%s", def->asset); + /* 2:Location */ + if (def->location) + virBufferAsprintf(&buf, ",location=%s", def->location); + + if (virBufferCheckError(&buf) < 0) + goto error; + + return virBufferContentAndReset(&buf); + + error: + virBufferFreeAndReset(&buf); + return NULL; +} + static char * qemuBuildClockArgStr(virDomainClockDefPtr def) { @@ -9057,6 +9096,21 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); VIR_FREE(smbioscmd); } + + if (source->nbaseBoard > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("qemu does not support more than " + "one entry to Type 2 in SMBIOS table")); + goto error; + } + + for (i = 0; i < source->nbaseBoard; i++) { + if (!(smbioscmd = qemuBuildSmbiosBaseBoardStr(source->baseBoard + i))) + goto error; + + virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); + VIR_FREE(smbioscmd); + } } }
diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 692c921..a4d31c9 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -91,6 +91,18 @@ void virSysinfoSystemDefFree(virSysinfoSystemDefPtr def) VIR_FREE(def); }
+void virSysinfoBaseBoardDefClear(virSysinfoBaseBoardDefPtr def)
Two lines and now can be static I believe Remainder seems fine - thanks for the examples
+{ + if (def == NULL) + return; + + VIR_FREE(def->manufacturer); + VIR_FREE(def->product); + VIR_FREE(def->version); + VIR_FREE(def->serial); + VIR_FREE(def->asset); + VIR_FREE(def->location); +}
/** * virSysinfoDefFree: @@ -109,6 +121,10 @@ void virSysinfoDefFree(virSysinfoDefPtr def) virSysinfoBIOSDefFree(def->bios); virSysinfoSystemDefFree(def->system);
+ for (i = 0; i < def->nbaseBoard; i++) + virSysinfoBaseBoardDefClear(def->baseBoard + i); + VIR_FREE(def->baseBoard); + for (i = 0; i < def->nprocessor; i++) { VIR_FREE(def->processor[i].processor_socket_destination); VIR_FREE(def->processor[i].processor_type); @@ -717,6 +733,84 @@ virSysinfoParseSystem(const char *base, virSysinfoSystemDefPtr *system) }
static int +virSysinfoParseBaseBoard(const char *base, + virSysinfoBaseBoardDefPtr *baseBoard, + size_t *nbaseBoard) +{ + int ret = -1; + const char *cur, *eol = NULL; + virSysinfoBaseBoardDefPtr boards = NULL; + size_t nboards = 0; + char *board_type = NULL; + + while (base && (cur = strstr(base, "Base Board Information"))) { + virSysinfoBaseBoardDefPtr def; + + if (VIR_EXPAND_N(boards, nboards, 1) < 0) + goto cleanup; + + def = &boards[nboards - 1]; + + base = cur + 22; + if ((cur = strstr(base, "Manufacturer: ")) != NULL) { + cur += 14; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->manufacturer, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Product Name: ")) != NULL) { + cur += 14; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->product, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Version: ")) != NULL) { + cur += 9; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Serial Number: ")) != NULL) { + cur += 15; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->serial, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Asset Tag: ")) != NULL) { + cur += 11; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->asset, cur, eol - cur) < 0) + goto cleanup; + } + if ((cur = strstr(base, "Location In Chassis: ")) != NULL) { + cur += 21; + eol = strchr(cur, '\n'); + if (eol && VIR_STRNDUP(def->location, cur, eol - cur) < 0) + goto cleanup; + } + + if (!def->manufacturer && !def->product && !def->version && + !def->serial && !def->asset && !def->location) + nboards--; + } + + /* This is safe, as we can be only shrinking the memory */ + ignore_value(VIR_REALLOC_N(boards, nboards)); + + *baseBoard = boards; + *nbaseBoard = nboards; + boards = NULL; + nboards = 0; + ret = 0; + cleanup: + while (nboards--) + virSysinfoBaseBoardDefClear(&boards[nboards]); + VIR_FREE(boards); + VIR_FREE(board_type); + return ret; +} + +static int virSysinfoParseProcessor(const char *base, virSysinfoDefPtr ret) { const char *cur, *tmp_base; @@ -938,7 +1032,7 @@ virSysinfoRead(void) return NULL; }
- cmd = virCommandNewArgList(path, "-q", "-t", "0,1,4,17", NULL); + cmd = virCommandNewArgList(path, "-q", "-t", "0,1,2,4,17", NULL); VIR_FREE(path); virCommandSetOutputBuffer(cmd, &outbuf); if (virCommandRun(cmd, NULL) < 0) @@ -955,6 +1049,9 @@ virSysinfoRead(void) if (virSysinfoParseSystem(outbuf, &ret->system) < 0) goto error;
+ if (virSysinfoParseBaseBoard(outbuf, &ret->baseBoard, &ret->nbaseBoard) < 0) + goto error; + ret->nprocessor = 0; ret->processor = NULL; if (virSysinfoParseProcessor(outbuf, ret) < 0) @@ -1025,6 +1122,36 @@ virSysinfoSystemFormat(virBufferPtr buf, virSysinfoSystemDefPtr def) }
static void +virSysinfoBaseBoardFormat(virBufferPtr buf, + virSysinfoBaseBoardDefPtr baseBoard, + size_t nbaseBoard) +{ + virSysinfoBaseBoardDefPtr def; + size_t i; + + for (i = 0; i < nbaseBoard; i++) { + def = baseBoard + i; + + virBufferAddLit(buf, "<baseBoard>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<entry name='manufacturer'>%s</entry>\n", + def->manufacturer); + virBufferEscapeString(buf, "<entry name='product'>%s</entry>\n", + def->product); + virBufferEscapeString(buf, "<entry name='version'>%s</entry>\n", + def->version); + virBufferEscapeString(buf, "<entry name='serial'>%s</entry>\n", + def->serial); + virBufferEscapeString(buf, "<entry name='asset'>%s</entry>\n", + def->asset); + virBufferEscapeString(buf, "<entry name='location'>%s</entry>\n", + def->location); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</baseBoard>\n"); + } +} + +static void virSysinfoProcessorFormat(virBufferPtr buf, virSysinfoDefPtr def) { size_t i; @@ -1157,6 +1284,7 @@ virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def)
virSysinfoBIOSFormat(&childrenBuf, def->bios); virSysinfoSystemFormat(&childrenBuf, def->system); + virSysinfoBaseBoardFormat(&childrenBuf, def->baseBoard, def->nbaseBoard); virSysinfoProcessorFormat(&childrenBuf, def); virSysinfoMemoryFormat(&childrenBuf, def);
@@ -1241,12 +1369,40 @@ virSysinfoSystemIsEqual(virSysinfoSystemDefPtr src, return identical; }
+static bool +virSysinfoBaseBoardIsEqual(virSysinfoBaseBoardDefPtr src, + virSysinfoBaseBoardDefPtr dst) +{ + bool identical = false; + + if (!src && !dst) + return true; + + if ((src && !dst) || (!src && dst)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Target base board does not match source")); + goto cleanup; + } + + CHECK_FIELD(manufacturer, "base board vendor"); + CHECK_FIELD(product, "base board product"); + CHECK_FIELD(version, "base board version"); + CHECK_FIELD(serial, "base board serial"); + CHECK_FIELD(asset, "base board asset"); + CHECK_FIELD(location, "base board location"); + + identical = true; + cleanup: + return identical; +} + #undef CHECK_FIELD
bool virSysinfoIsEqual(virSysinfoDefPtr src, virSysinfoDefPtr dst) { bool identical = false; + size_t i;
if (!src && !dst) return true; @@ -1271,6 +1427,18 @@ bool virSysinfoIsEqual(virSysinfoDefPtr src, if (!virSysinfoSystemIsEqual(src->system, dst->system)) goto cleanup;
+ if (src->nbaseBoard != dst->nbaseBoard) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target sysinfo base board count '%zu' does not match source '%zu'"), + dst->nbaseBoard, src->nbaseBoard); + goto cleanup; + } + + for (i = 0; i < src->nbaseBoard; i++) + if (!virSysinfoBaseBoardIsEqual(src->baseBoard + i, + dst->baseBoard + i)) + goto cleanup; + identical = true;
cleanup: diff --git a/src/util/virsysinfo.h b/src/util/virsysinfo.h index c8cc1e8..1e51d2c 100644 --- a/src/util/virsysinfo.h +++ b/src/util/virsysinfo.h @@ -86,6 +86,18 @@ struct _virSysinfoSystemDef { char *family; };
+typedef struct _virSysinfoBaseBoardDef virSysinfoBaseBoardDef; +typedef virSysinfoBaseBoardDef *virSysinfoBaseBoardDefPtr; +struct _virSysinfoBaseBoardDef { + char *manufacturer; + char *product; + char *version; + char *serial; + char *asset; + char *location; + /* XXX board type */ +}; + typedef struct _virSysinfoDef virSysinfoDef; typedef virSysinfoDef *virSysinfoDefPtr; struct _virSysinfoDef { @@ -94,6 +106,9 @@ struct _virSysinfoDef { virSysinfoBIOSDefPtr bios; virSysinfoSystemDefPtr system;
+ size_t nbaseBoard; + virSysinfoBaseBoardDefPtr baseBoard; + size_t nprocessor; virSysinfoProcessorDefPtr processor;
@@ -105,6 +120,7 @@ virSysinfoDefPtr virSysinfoRead(void);
void virSysinfoBIOSDefFree(virSysinfoBIOSDefPtr def); void virSysinfoSystemDefFree(virSysinfoSystemDefPtr def); +void virSysinfoBaseBoardDefClear(virSysinfoBaseBoardDefPtr def); void virSysinfoDefFree(virSysinfoDefPtr def);
int virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios-multiple-type2.xml b/tests/qemuxml2argvdata/qemuxml2argv-smbios-multiple-type2.xml new file mode 100644 index 0000000..1f6aec1 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios-multiple-type2.xml @@ -0,0 +1,58 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <sysinfo type='smbios'> + <bios> + <entry name='vendor'>LENOVO</entry> + <entry name='version'>6FET82WW (3.12 )</entry> + </bios> + <system> + <entry name='manufacturer'>Fedora</entry> + <entry name='product'>Virt-Manager</entry> + <entry name='version'>0.8.2-3.fc14</entry> + <entry name='serial'>32dfcb37-5af1-552b-357c-be8c3aa38310</entry> + <entry name='uuid'>c7a5fdbd-edaf-9455-926a-d65c16db1809</entry> + <entry name='sku'>1234567890</entry> + <entry name='family'>Red Hat</entry> + </system> + <baseBoard> + <entry name='manufacturer'>Hewlett-Packard</entry> + <entry name='product'>0B4Ch</entry> + <entry name='version'>D</entry> + <entry name='serial'>CZC1065993</entry> + <entry name='asset'>CZC1065993</entry> + <entry name='location'>Upside down</entry> + </baseBoard> + <baseBoard> + <entry name='manufacturer'>Lenovo</entry> + <entry name='product'>20BE0061MC</entry> + <entry name='version'>0B98401 Pro</entry> + <entry name='serial'>W1KS427111E</entry> + <entry name='location'>Not Available</entry> + </baseBoard> + </sysinfo> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + <smbios mode='sysinfo'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args index e939aca..209cf20 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args @@ -4,5 +4,7 @@ pc -m 214 -smp 1 -smbios 'type=0,vendor=LENOVO,version=6FET82WW (3.12 )' \ -smbios 'type=1,manufacturer=Fedora,product=Virt-Manager,version=0.8.2-3.fc14,\ serial=32dfcb37-5af1-552b-357c-be8c3aa38310,\ uuid=c7a5fdbd-edaf-9455-926a-d65c16db1809,sku=1234567890,family=Red Hat' \ +-smbios 'type=2,manufacturer=Hewlett-Packard,product=0B4Ch,version=D,\ +serial=CZC1065993,asset=CZC1065993,location=Upside down' \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml b/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml index a2caeea..30888ae 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml @@ -18,6 +18,14 @@ <entry name='sku'>1234567890</entry> <entry name='family'>Red Hat</entry> </system> + <baseBoard> + <entry name='manufacturer'>Hewlett-Packard</entry> + <entry name='product'>0B4Ch</entry> + <entry name='version'>D</entry> + <entry name='serial'>CZC1065993</entry> + <entry name='asset'>CZC1065993</entry> + <entry name='location'>Upside down</entry> + </baseBoard> </sysinfo> <os> <type arch='i686' machine='pc'>hvm</type> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 44b388c..3287ea3 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -614,6 +614,7 @@ mymain(void) DO_TEST_DIFFERENT("tap-vhost-incorrect"); DO_TEST("shmem"); DO_TEST("smbios"); + DO_TEST("smbios-multiple-type2"); DO_TEST("aarch64-aavmf-virtio-mmio");
DO_TEST("memory-hotplug");
On 17.06.2015 15:44, John Ferlan wrote:
On 06/12/2015 06:02 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1220527
This type of information defines attributes of a system baseboard. With one caveat: in qemu they call it family, while in the specification it's referred to as type. I'm sticking with the latter.
Perhaps should update this since 'family' and 'type' aren't processed by libvirt.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> ---
diff to v1: -I've dropped the 'family' attribute. It's not implemented in qemu yet. We can add it later.
docs/formatdomain.html.in | 37 ++++- docs/schemas/domaincommon.rng | 23 +++ src/conf/domain_conf.c | 63 ++++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 54 +++++++ src/util/virsysinfo.c | 170 ++++++++++++++++++++- src/util/virsysinfo.h | 16 ++ .../qemuxml2argv-smbios-multiple-type2.xml | 58 +++++++ tests/qemuxml2argvdata/qemuxml2argv-smbios.args | 2 + tests/qemuxml2argvdata/qemuxml2argv-smbios.xml | 8 + tests/qemuxml2xmltest.c | 1 + 11 files changed, 427 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-smbios-multiple-type2.xml
There's a couple issues pointed out below, fixable without need for a v3 I believe...
ACK with the adjustments
John
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0478cb2..977660e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -375,6 +375,12 @@ <entry name='product'>Virt-Manager</entry> <entry name='version'>0.9.4</entry> </system> + <baseBoard> + <entry name='manufacturer'>LENOVO</entry> + <entry name='product'>20BE0061MC</entry> + <entry name='version'>0B98401 Pro</entry> + <entry name='serial'>W1KS427111E</entry> + </baseBoard> </sysinfo> ...</pre>
@@ -435,11 +441,32 @@ <dt><code>family</code></dt> <dd>Identify the family a particular computer belongs to.</dd> </dl> - NB: Incorrectly supplied entries in either the <code>bios</code> - or <code>system</code> blocks will be ignored without error. - Other than <code>uuid</code> validation and <code>date</code> - format checking, all values are passed as strings to the - hypervisor driver. + </dd> + <dt><code>baseBoard</code></dt> + <dd> + This is block 2 of SMBIOS. This element can be repeated multiple + times, to describe all the base boards. However, not all
s/times,/times s/boards. However,/boards; however,
+ hypervisors support the repetition necessarily. The element can
s/support the repetition necessarily/necessarily support the repitition
or
s/support the repetition necessarily./support multiple base boards./
+ have the following children: + <dl> + <dt><code>manufacturer</code></dt> + <dd>Manufacturer of BIOS</dd> + <dt><code>product</code></dt> + <dd>Product Name</dd> + <dt><code>version</code></dt> + <dd>Version of the product</dd> + <dt><code>serial</code></dt> + <dd>Serial number</dd> + <dt><code>asset</code></dt> + <dd>Asset tag</dd> + <dt><code>location</code></dt> + <dd>Location in chassis</dd> + </dl> + NB: Incorrectly supplied entries for the + <code>bios</code>, <code>system</code> or <code>baseBoard</code> + blocks will be ignored without error. Other than <code>uuid</code> + validation and <code>date</code> format checking, all values are + passed as strings to the hypervisor driver. </dd> </dl> </dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index e38b927..32d28cd 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4248,6 +4248,18 @@ </oneOrMore> </element> </optional> + <zeroOrMore> + <element name="baseBoard"> + <oneOrMore> + <element name="entry"> + <attribute name="name"> + <ref name="sysinfo-baseBoard-name"/> + </attribute> + <ref name="sysinfo-value"/> + </element> + </oneOrMore> + </element> + </zeroOrMore> </interleave> </element> </define> @@ -4273,6 +4285,17 @@ </choice> </define>
+ <define name="sysinfo-baseBoard-name"> + <choice> + <value>manufacturer</value> + <value>product</value> + <value>version</value> + <value>serial</value> + <value>asset</value> + <value>location</value> + </choice> + </define> + <define name="sysinfo-value"> <data type="string"> <param name='pattern'>[a-zA-Z0-9/\-_\. \(\)]+</param> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bf7eeb2..c2174d9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11227,6 +11227,65 @@ virSysinfoSystemParseXML(xmlNodePtr node, return ret; }
+static int +virSysinfoBaseBoardParseXML(xmlXPathContextPtr ctxt, + virSysinfoBaseBoardDefPtr *baseBoard, + size_t *nbaseBoard) +{ + int ret = -1; + virSysinfoBaseBoardDefPtr boards = NULL; + size_t i, nboards = 0; + char *board_type = NULL;
Unused
Good catch! I've split v1 into two patches locally. I've extracted the board type into a separate patch but this has somehow slipped through.
+ xmlNodePtr *nodes = NULL, oldnode = ctxt->node; + int n; + + if ((n = virXPathNodeSet("./baseBoard", ctxt, &nodes)) < 0) + return ret; + + if (n && VIR_ALLOC_N(boards, n) < 0) + goto cleanup; + + for (i = 0; i < n; i++) { + virSysinfoBaseBoardDefPtr def = boards + nboards; + + ctxt->node = nodes[i]; + + def->manufacturer = + virXPathString("string(entry[@name='manufacturer'])", ctxt); + def->product = + virXPathString("string(entry[@name='product'])", ctxt); + def->version = + virXPathString("string(entry[@name='version'])", ctxt); + def->serial = + virXPathString("string(entry[@name='serial'])", ctxt); + def->asset = + virXPathString("string(entry[@name='asset'])", ctxt); + def->location = + virXPathString("string(entry[@name='location'])", ctxt); + + if (!def->manufacturer && !def->product && !def->version && + !def->serial && !def->asset && !def->location) { + /* nada */ + } else { + nboards++; + } + } + + *baseBoard = boards; + *nbaseBoard = nboards; + boards = NULL; + nboards = 0; + ret = 0; + cleanup: + while (nboards--) + virSysinfoBaseBoardDefClear(&boards[nboards]);
Coverity notes nboards can never be anything other than zero when we get here. IOW we can never jump out of that for loop where nboards gets incremented unlike virSysinfoParseBaseBoard where the VIR_EXPAND_N failure...
Yeah, now that we are not trying to fit a string into an enum (= parse board type), these two lines do not make much sense. I'll move them to the other patch.
+ VIR_FREE(boards); + VIR_FREE(board_type);
Unused
+ VIR_FREE(nodes); + ctxt->node = oldnode; + return ret; +} + static virSysinfoDefPtr virSysinfoParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -11281,6 +11340,10 @@ virSysinfoParseXML(xmlNodePtr node, ctxt->node = oldnode; }
+ /* Extract system base board metadata */ + if (virSysinfoBaseBoardParseXML(ctxt, &def->baseBoard, &def->nbaseBoard) < 0) + goto error; + cleanup: VIR_FREE(type); return def; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dc8a52d..2348f14 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2182,6 +2182,7 @@ virVasprintfInternal;
# util/virsysinfo.h +virSysinfoBaseBoardDefClear;
Probably won't be necessary now that domain_conf.c doesn't need it...
Well, strictly speaking it's not needed to be exported right now, correct. However, I think it's more future-proof if we export it. I mean, in my scenario, if this was not exported, I'd need yet another patch that will just export it. Thanks for the review! Michal
participants (2)
-
John Ferlan -
Michal Privoznik