--- docs/formatdomain.html.in | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 9ade507..825053c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -308,7 +308,8 @@ &lt;/bios&gt; &lt;system&gt; &lt;entry name='manufacturer'&gt;Fedora&lt;/entry&gt; - &lt;entry name='vendor'&gt;Virt-Manager&lt;/entry&gt; + &lt;entry name='product'&gt;Virt-Manager&lt;/entry&gt; + &lt;entry name='version'&gt;0.9.4&lt;/entry&gt; &lt;/system&gt; &lt;/sysinfo&gt; ...</pre> -- 1.8.1.4

--- docs/formatdomain.html.in | 48 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 8 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 825053c..abf14fa 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -333,17 +333,49 @@ <dl> <dt><code>bios</code></dt> <dd> - This is block 0 of SMBIOS, with entry names drawn from - "vendor", "version", "date", and "release". + This is block 0 of SMBIOS, with entry names drawn from: + <dl> + <dt><code>vendor</code></dt> + <dd>BIOS Vendors Name</dd> + <dt><code>version</code></dt> + <dd>BIOS Version</dd> + <dt><code>date</code></dt> + <dd>BIOS release date. If supplied, is in either mm/dd/yy or + mm/dd/yyyy format. If the year portion of the string is + two digits, the year is assumed to be 19yy.</dd> + <dt><code>release</code></dt> + <dd>System BIOS Major and Minor release number values + concatenated together as one string separated by + a period, for example, 10.22.</dd> + </dl> </dd> <dt><code>system</code></dt> <dd> - This is block 1 of SMBIOS, with entry names drawn from - "manufacturer", "product", "version", "serial", "uuid", - "sku", and "family". If a "uuid" entry is provided - alongside a - top-level <a href="#elementsMetadata"><code>uuid</code> - element</a>, the two values must match. + This is block 1 of SMBIOS, with entry names drawn from: + <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>uuid</code></dt> + <dd>Universal Unique ID number. If this entry is provided + alongside a top-level + <a href="#elementsMetadata"><code>uuid</code></a> element, + then the two values must match.</dd> + <dt><code>sku</code></dt> + <dd>SKU number to identify a particular configuration.</dd> + <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> </dl> </dd> -- 1.8.1.4

Add incorrectly formatted bios_date validation test --- src/conf/domain_conf.c | 24 ++++++++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-smbios-date.xml | 23 +++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 48 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-smbios-date.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 862b997..d352055 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8437,6 +8437,30 @@ virSysinfoParseXML(const xmlNodePtr node, virXPathString("string(bios/entry[@name='version'])", ctxt); def->bios_date = virXPathString("string(bios/entry[@name='date'])", ctxt); + if (def->bios_date != NULL) { + char *ptr; + struct tm tm; + memset(&tm, 0, sizeof(tm)); + + /* Validate just the format of the date + * Expect mm/dd/yyyy or mm/dd/yy, + * where yy must be 00->99 and would be assumed to be 19xx + * a yyyy date should be 1900 and beyond + */ + if (virStrToLong_i(def->bios_date, &ptr, 10, &tm.tm_mon) < 0 || + *ptr != '/' || + virStrToLong_i(ptr+1, &ptr, 10, &tm.tm_mday) < 0 || + *ptr != '/' || + virStrToLong_i(ptr+1, &ptr, 10, &tm.tm_year) < 0 || + *ptr != '\0' || + (tm.tm_mon < 0 || tm.tm_mon > 12) || + (tm.tm_mday < 0 || tm.tm_mday > 31) || + (tm.tm_year < 0 || (tm.tm_year >= 100 && tm.tm_year < 1900))) { + virReportError(VIR_ERR_XML_DETAIL, "%s", + _("Invalid BIOS 'date' format")); + goto error; + } + } def->bios_release = virXPathString("string(bios/entry[@name='release'])", ctxt); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios-date.xml b/tests/qemuxml2argvdata/qemuxml2argv-smbios-date.xml new file mode 100644 index 0000000..7b2f33a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios-date.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>smbios</name> + <uuid>362d1fc1-df7d-193e-5c18-49a71bd1da66</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <smbios mode="sysinfo"/> + </os> + <sysinfo type="smbios"> + <bios> + <entry name="date">999/999/123</entry> + </bios> + </sysinfo> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 98ceb83..0100937 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -815,6 +815,7 @@ mymain(void) DO_TEST("smbios", QEMU_CAPS_SMBIOS_TYPE); + DO_TEST_PARSE_ERROR("smbios-date", QEMU_CAPS_SMBIOS_TYPE); DO_TEST("watchdog", NONE); DO_TEST("watchdog-device", QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); -- 1.8.1.4

On 05/13/2013 03:51 PM, Eric Blake wrote: Oh right - at one time I was doing the gmtime or strptime call to attempt to validate the data; however, I found it was less than perfect allowing things like 2/31/2000 to be accepted. In any case, in lieu of a v3, here's a diff: diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fbe4d9a..0516413 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8442,23 +8442,22 @@ virSysinfoParseXML(const xmlNodePtr node, virXPathString("string(bios/entry[@name='date'])", ctxt); if (def->bios_date != NULL) { char *ptr; - struct tm tm; - memset(&tm, 0, sizeof(tm)); + int month, day, year; /* Validate just the format of the date * Expect mm/dd/yyyy or mm/dd/yy, * where yy must be 00->99 and would be assumed to be 19xx * a yyyy date should be 1900 and beyond */ - if (virStrToLong_i(def->bios_date, &ptr, 10, &tm.tm_mon) < 0 || + if (virStrToLong_i(def->bios_date, &ptr, 10, &month) < 0 || *ptr != '/' || - virStrToLong_i(ptr+1, &ptr, 10, &tm.tm_mday) < 0 || + virStrToLong_i(ptr + 1, &ptr, 10, &day) < 0 || *ptr != '/' || - virStrToLong_i(ptr+1, &ptr, 10, &tm.tm_year) < 0 || + virStrToLong_i(ptr + 1, &ptr, 10, &year) < 0 || *ptr != '\0' || - (tm.tm_mon < 0 || tm.tm_mon > 12) || - (tm.tm_mday < 0 || tm.tm_mday > 31) || - (tm.tm_year < 0 || (tm.tm_year >= 100 && tm.tm_year < 1900))) { + (month < 1 || month > 12) || + (day < 1 || day > 31) || + (year < 0 || (year >= 100 && year < 1900))) { virReportError(VIR_ERR_XML_DETAIL, "%s", _("Invalid BIOS 'date' format")); goto error; or more visually appealing if (def->bios_date != NULL) { char *ptr; int month, day, year; /* Validate just the format of the date * Expect mm/dd/yyyy or mm/dd/yy, * where yy must be 00->99 and would be assumed to be 19xx * a yyyy date should be 1900 and beyond */ if (virStrToLong_i(def->bios_date, &ptr, 10, &month) < 0 || *ptr != '/' || virStrToLong_i(ptr + 1, &ptr, 10, &day) < 0 || *ptr != '/' || virStrToLong_i(ptr + 1, &ptr, 10, &year) < 0 || *ptr != '\0' || (month < 1 || month > 12) || (day < 1 || day > 31) || (year < 0 || (year >= 100 && year < 1900))) { virReportError(VIR_ERR_XML_DETAIL, "%s", _("Invalid BIOS 'date' format")); goto error; } } John

If the <sysinfo> system table 'uuid' field is improperly formatted, then qemu will fail to start the guest with the error: virsh start dom error: Failed to start domain dom error: internal error process exited while connecting to monitor: Invalid SMBIOS UUID string This was because the parsing rules were lax with respect to allowing extraneous spaces and dashes in the provided UUID. As long as there were 32 hexavalues that matched the UUID for the domain the string was accepted. However startup failed because the string format wasn't correct. This patch will adjust the string format so that when it's presented to the driver it's in the expected format. Addded a test for uuid comparison within sysinfo. --- src/conf/domain_conf.c | 54 ++++++++++++++-------- .../qemuxml2argv-smbios-uuid-match.xml | 23 +++++++++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 58 insertions(+), 20 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-smbios-uuid-match.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d352055..fbe4d9a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8401,10 +8401,13 @@ error: static virSysinfoDefPtr virSysinfoParseXML(const xmlNodePtr node, - xmlXPathContextPtr ctxt) + xmlXPathContextPtr ctxt, + unsigned char *domUUID, + bool uuid_generated) { virSysinfoDefPtr def; char *type; + char *tmpUUID = NULL; if (!xmlStrEqual(node->name, BAD_CAST "sysinfo")) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -8473,8 +8476,33 @@ virSysinfoParseXML(const xmlNodePtr node, virXPathString("string(system/entry[@name='version'])", ctxt); def->system_serial = virXPathString("string(system/entry[@name='serial'])", ctxt); - def->system_uuid = - virXPathString("string(system/entry[@name='uuid'])", ctxt); + tmpUUID = virXPathString("string(system/entry[@name='uuid'])", ctxt); + if (tmpUUID) { + unsigned char uuidbuf[VIR_UUID_BUFLEN]; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + if (virUUIDParse(tmpUUID, uuidbuf) < 0) { + virReportError(VIR_ERR_XML_DETAIL, + "%s", _("malformed <sysinfo> uuid element")); + goto error; + } + if (uuid_generated) + memcpy(domUUID, uuidbuf, VIR_UUID_BUFLEN); + else if (memcmp(domUUID, uuidbuf, VIR_UUID_BUFLEN) != 0) { + virReportError(VIR_ERR_XML_DETAIL, "%s", + _("UUID mismatch between <uuid> and " + "<sysinfo>")); + goto error; + } + /* Although we've validated the UUID as good, virUUIDParse() is + * lax with respect to allowing extraneous "-" and " ", but the + * underlying hypervisor may be less forgiving. Use virUUIDFormat() + * to validate format in xml is right. If not, then format it + * properly so that it's used correctly later. + */ + virUUIDFormat(uuidbuf, uuidstr); + if (VIR_STRDUP(def->system_uuid, uuidstr) < 0) + goto error; + } def->system_sku = virXPathString("string(system/entry[@name='sku'])", ctxt); def->system_family = @@ -8482,6 +8510,7 @@ virSysinfoParseXML(const xmlNodePtr node, cleanup: VIR_FREE(type); + VIR_FREE(tmpUUID); return def; error: @@ -11737,27 +11766,12 @@ virDomainDefParseXML(xmlDocPtr xml, if ((node = virXPathNode("./sysinfo[1]", ctxt)) != NULL) { xmlNodePtr oldnode = ctxt->node; ctxt->node = node; - def->sysinfo = virSysinfoParseXML(node, ctxt); + def->sysinfo = virSysinfoParseXML(node, ctxt, + def->uuid, uuid_generated); ctxt->node = oldnode; if (def->sysinfo == NULL) goto error; - if (def->sysinfo->system_uuid != NULL) { - unsigned char uuidbuf[VIR_UUID_BUFLEN]; - if (virUUIDParse(def->sysinfo->system_uuid, uuidbuf) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("malformed uuid element")); - goto error; - } - if (uuid_generated) - memcpy(def->uuid, uuidbuf, VIR_UUID_BUFLEN); - else if (memcmp(def->uuid, uuidbuf, VIR_UUID_BUFLEN) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("UUID mismatch between <uuid> and " - "<sysinfo>")); - goto error; - } - } } if ((tmp = virXPathString("string(./os/smbios/@mode)", ctxt))) { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios-uuid-match.xml b/tests/qemuxml2argvdata/qemuxml2argv-smbios-uuid-match.xml new file mode 100644 index 0000000..322fb3e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios-uuid-match.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>smbios</name> + <uuid>362d1fc1-df7d-193e-5c18-49a71bd1da66</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <smbios mode="sysinfo"/> + </os> + <sysinfo type="smbios"> + <system> + <entry name="uuid">a94b4335-6a14-8bc4-d6da-f7ea590b6816</entry> + </system> + </sysinfo> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 0100937..92c72e0 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -816,6 +816,7 @@ mymain(void) DO_TEST("smbios", QEMU_CAPS_SMBIOS_TYPE); DO_TEST_PARSE_ERROR("smbios-date", QEMU_CAPS_SMBIOS_TYPE); + DO_TEST_PARSE_ERROR("smbios-uuid-match", QEMU_CAPS_SMBIOS_TYPE); DO_TEST("watchdog", NONE); DO_TEST("watchdog-device", QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); -- 1.8.1.4