[libvirt] [PATCH 0/2] smbios fixes
In testing <smbios mode='host'/>, I noticed a couple of problems. First, qemu rejected the command line we gave it (invalid UUID); ifixingthat showed that all other arguments were being given literal "" which then made the guest smbios differ from the host. Second, the qemu option -smbios type=1,family=string wasn't supported, which lead to a spurious difference between host and guest. Meanwhile, qemu has a bug where '-smbios type=1,uuid=uuid' must parse as a valid uuid, but is otherwise ignored. The current qemu.git code base _only_ sets smbios uuid from the '-uuid uuid' argument. I've filed a bug against the qemu folks asking whether this is intended (in which case we have to modify libvirt to change the -uuid argument it outputs when smbios is specified), or an oversight (hopefully the latter, since it's still nice to correlate /var/log/libvirt/qemu/....log with the XML uuid even when that differs from the smbios uuid presented to the guest.) Eric Blake (2): qemu: avoid adding "" in smbios arguments smbios: support system family docs/schemas/domain.rng | 1 + src/conf/domain_conf.c | 9 +++++++- src/qemu/qemu_conf.c | 26 +++++++++++++--------- src/util/sysinfo.c | 7 ++++++ src/util/sysinfo.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-smbios.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-smbios.xml | 2 + 7 files changed, 35 insertions(+), 13 deletions(-)
The log lists things like -smbios type=1,vendor="Red Hat", which is great for shell parsing, but not so great when you realize that execve() then passes those literal "" on as part of the command line argument, such that qemu sets SMBIOS with extra literal quotes. The eventual addition of virCommand is needed before we have the API to shell-quote a string representation of a command line, so that the log can still be pasted into a shell, but without inserting extra bytes into the execve() arguments. * src/qemu/qemu_conf.c (qemuBuildSmbiosBiosStr) (qemuBuildSmbiosSystemStr): Qemu doesn't like quotes around uuid arguments, and the remaining quotes are passed literally to smbios, making <smbios mode='host'/> inaccurate. Removing the quotes makes the log harder to parse, but that can be fixed later with virCommand improvements. * tests/qemuxml2argvdata/qemuxml2argv-smbios.args: 'Fix' test; it will need fixing again once virCommand learns how to shell-quote a potential command line. --- src/qemu/qemu_conf.c | 20 ++++++++++---------- tests/qemuxml2argvdata/qemuxml2argv-smbios.args | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 7cd0603..50c1e6c 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -3613,16 +3613,16 @@ static char *qemuBuildSmbiosBiosStr(virSysinfoDefPtr def) /* 0:Vendor */ if (def->bios_vendor) - virBufferVSprintf(&buf, ",vendor=\"%s\"", def->bios_vendor); + virBufferVSprintf(&buf, ",vendor=%s", def->bios_vendor); /* 0:BIOS Version */ if (def->bios_version) - virBufferVSprintf(&buf, ",version=\"%s\"", def->bios_version); + virBufferVSprintf(&buf, ",version=%s", def->bios_version); /* 0:BIOS Release Date */ if (def->bios_date) - virBufferVSprintf(&buf, ",date=\"%s\"", def->bios_date); + virBufferVSprintf(&buf, ",date=%s", def->bios_date); /* 0:System BIOS Major Release and 0:System BIOS Minor Release */ if (def->bios_release) - virBufferVSprintf(&buf, ",release=\"%s\"", def->bios_release); + virBufferVSprintf(&buf, ",release=%s", def->bios_release); if (virBufferError(&buf)) { virReportOOMError(); @@ -3649,23 +3649,23 @@ static char *qemuBuildSmbiosSystemStr(virSysinfoDefPtr def) /* 1:Manufacturer */ if (def->system_manufacturer) - virBufferVSprintf(&buf, ",manufacturer=\"%s\"", + virBufferVSprintf(&buf, ",manufacturer=%s", def->system_manufacturer); /* 1:Product Name */ if (def->system_product) - virBufferVSprintf(&buf, ",product=\"%s\"", def->system_product); + virBufferVSprintf(&buf, ",product=%s", def->system_product); /* 1:Version */ if (def->system_version) - virBufferVSprintf(&buf, ",version=\"%s\"", def->system_version); + virBufferVSprintf(&buf, ",version=%s", def->system_version); /* 1:Serial Number */ if (def->system_serial) - virBufferVSprintf(&buf, ",serial=\"%s\"", def->system_serial); + virBufferVSprintf(&buf, ",serial=%s", def->system_serial); /* 1:UUID */ if (def->system_uuid) - virBufferVSprintf(&buf, ",uuid=\"%s\"", def->system_uuid); + virBufferVSprintf(&buf, ",uuid=%s", def->system_uuid); /* 1:SKU Number */ if (def->system_sku) - virBufferVSprintf(&buf, ",sku=\"%s\"", def->system_sku); + virBufferVSprintf(&buf, ",sku=%s", def->system_sku); if (virBufferError(&buf)) { virReportOOMError(); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args index d5bd289..bd3ede4 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -smbios type=0,vendor="QEmu/KVM",version="0.13" -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" -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -smbios type=0,vendor=QEmu/KVM,version=0.13 -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 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -- 1.7.1
* docs/schemas/domain.rng (sysinfo-system-name): Also allow family. * src/util/sysinfo.h (struct _virSysinfoDef): Add system_family. * src/conf/domain_conf.c (virSysinfoParseXML) (virDomainSysinfoDefFormat): Support it. * src/util/sysinfo.c (virSysinfoDefFree, virSysinfoRead): Likewise. * src/qemu/qemu_conf.c (qemuBuildSmbiosSystemStr): Likewise. * tests/qemuxml2argvdata/qemuxml2argv-smbios.xml: Adjust test. * tests/qemuxml2argvdata/qemuxml2argv-smbios.args: Likewise. --- docs/schemas/domain.rng | 1 + src/conf/domain_conf.c | 9 ++++++++- src/qemu/qemu_conf.c | 6 +++++- src/util/sysinfo.c | 7 +++++++ src/util/sysinfo.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-smbios.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-smbios.xml | 2 ++ 7 files changed, 25 insertions(+), 3 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index fb44335..6c47ea0 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1873,6 +1873,7 @@ <value>serial</value> <value>uuid</value> <value>sku</value> + <value>family</value> </choice> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3f14cee..91d7cce 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3626,6 +3626,8 @@ virSysinfoParseXML(const xmlNodePtr node, virXPathString("string(system/entry[@name='uuid'])", ctxt); def->system_sku = virXPathString("string(system/entry[@name='sku'])", ctxt); + def->system_family = + virXPathString("string(system/entry[@name='family'])", ctxt); cleanup: VIR_FREE(type); @@ -6426,7 +6428,8 @@ virDomainSysinfoDefFormat(virBufferPtr buf, } if ((def->system_manufacturer != NULL) || (def->system_product != NULL) || (def->system_version != NULL) || (def->system_serial != NULL) || - (def->system_uuid != NULL) || (def->system_sku != NULL)) { + (def->system_uuid != NULL) || (def->system_sku != NULL) || + (def->system_family != NULL)) { virBufferAddLit(buf, " <system>\n"); if (def->system_manufacturer != NULL) virBufferEscapeString(buf, @@ -6452,6 +6455,10 @@ virDomainSysinfoDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " <entry name='sku'>%s</entry>\n", def->system_sku); + if (def->system_family != NULL) + virBufferEscapeString(buf, + " <entry name='family'>%s</entry>\n", + def->system_family); virBufferAddLit(buf, " </system>\n"); } diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 50c1e6c..ad4d4fc 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -3642,7 +3642,8 @@ static char *qemuBuildSmbiosSystemStr(virSysinfoDefPtr def) if ((def->system_manufacturer == NULL) && (def->system_sku == NULL) && (def->system_product == NULL) && (def->system_uuid == NULL) && - (def->system_version == NULL) && (def->system_serial == NULL)) + (def->system_version == NULL) && (def->system_serial == NULL) && + (def->system_family == NULL)) return(NULL); virBufferAddLit(&buf, "type=1"); @@ -3666,6 +3667,9 @@ static char *qemuBuildSmbiosSystemStr(virSysinfoDefPtr def) /* 1:SKU Number */ if (def->system_sku) virBufferVSprintf(&buf, ",sku=%s", def->system_sku); + /* 1:Family */ + if (def->system_family) + virBufferVSprintf(&buf, ",family=%s", def->system_family); if (virBufferError(&buf)) { virReportOOMError(); diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c index 8ad98fe..cf41773 100644 --- a/src/util/sysinfo.c +++ b/src/util/sysinfo.c @@ -68,6 +68,7 @@ void virSysinfoDefFree(virSysinfoDefPtr def) VIR_FREE(def->system_serial); VIR_FREE(def->system_uuid); VIR_FREE(def->system_sku); + VIR_FREE(def->system_family); VIR_FREE(def); } @@ -217,6 +218,12 @@ virSysinfoRead(void) { if ((eol) && ((ret->system_sku = strndup(cur, eol - cur)) == NULL)) goto no_memory; } + if ((cur = strstr(base, "Family: ")) != NULL) { + cur += 8; + eol = strchr(cur, '\n'); + if ((eol) && ((ret->system_family = strndup(cur, eol - cur)) == NULL)) + goto no_memory; + } cleanup: VIR_FREE(outbuf); diff --git a/src/util/sysinfo.h b/src/util/sysinfo.h index 611d54e..1af7ef6 100644 --- a/src/util/sysinfo.h +++ b/src/util/sysinfo.h @@ -49,6 +49,7 @@ struct _virSysinfoDef { char *system_serial; char *system_uuid; char *system_sku; + char *system_family; }; virSysinfoDefPtr virSysinfoRead(void); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args index bd3ede4..b5e4783 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -smbios type=0,vendor=QEmu/KVM,version=0.13 -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 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -smbios type=0,vendor=QEmu/KVM,version=0.13 -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 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml b/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml index 66cbbb0..45b6dba 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml @@ -15,6 +15,8 @@ <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> </sysinfo> <os> -- 1.7.1
On 12/01/2010 05:12 PM, Eric Blake wrote:
* docs/schemas/domain.rng (sysinfo-system-name): Also allow family.
Unfortunately, <sysinfo> and <smbios> were never documented in docs/formatdomain.html.in; we'll have to add that before we can add <entry name='family'>string</entry> to that documentation.
+++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml @@ -15,6 +15,8 @@ <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>
Don't we typically use '' rather than "" for attribute values? We should probably fix that to be consistent as well. Another problem I just noticed - I can't roundtrip from <smbios mode='host'/> back to the full-blown <smbios mode='sysinfo'/> on my laptop, because my laptop has the following dmidecode entries: Handle 0x0000, DMI type 0, 24 bytes Vendor: LENOVO Version: 6FET82WW (3.12 ) Release Date: 11/26/2009 BIOS Revision: 3.18 Handle 0x0100, DMI type 1, 27 bytes Manufacturer: LENOVO Product Name: 2241B36 Version: ThinkPad T500 Serial Number: R89055N UUID: E321ED02-FAC8-9337-B0E7-C3B32580E899 SKU Number: Not Specified Family: Red Hat Enterprise Linux but the schema only allows: <define name="sysinfo-value"> <data type="string"> <param name='pattern'>[a-zA-Z0-9/\-_\. ]+</param> </data> </define> which rejects my Version string in block 0. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
* docs/schemas/domain.rng (sysinf-value): Expand pattern. * tests/qemuxml2argvdata/qemuxml2argv-smbios.xml: Prefer '' over "" for attribute values. Copy real hardware values. * tests/qemuxml2argvdata/qemuxml2argv-smbios.args: Likewise. --- This should address some of the issues I raised in https://www.redhat.com/archives/libvir-list/2010-December/msg00086.html (still missing is docs/formatdomain.html.in for all of this). docs/schemas/domain.rng | 2 +- tests/qemuxml2argvdata/qemuxml2argv-smbios.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-smbios.xml | 22 +++++++++++----------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 6c47ea0..d08d763 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1879,7 +1879,7 @@ <define name="sysinfo-value"> <data type="string"> - <param name='pattern'>[a-zA-Z0-9/\-_\. ]+</param> + <param name='pattern'>[a-zA-Z0-9/\-_\. \(\)]+</param> </data> </define> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args index b5e4783..12ef04f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -smbios type=0,vendor=QEmu/KVM,version=0.13 -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 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M 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 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml b/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml index 45b6dba..23ec1a7 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml @@ -4,25 +4,25 @@ <memory>219200</memory> <currentMemory>219200</currentMemory> <vcpu>1</vcpu> - <sysinfo type="smbios"> + <sysinfo type='smbios'> <bios> - <entry name="vendor">QEmu/KVM</entry> - <entry name="version">0.13</entry> + <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> + <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> </sysinfo> <os> <type arch='i686' machine='pc'>hvm</type> <boot dev='hd'/> - <smbios mode="sysinfo"/> + <smbios mode='sysinfo'/> </os> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> -- 1.7.1
On Wed, Dec 01, 2010 at 05:12:08PM -0700, Eric Blake wrote:
In testing <smbios mode='host'/>, I noticed a couple of problems. First, qemu rejected the command line we gave it (invalid UUID); ifixingthat showed that all other arguments were being given literal "" which then made the guest smbios differ from the host. Second, the qemu option -smbios type=1,family=string wasn't supported, which lead to a spurious difference between host and guest.
Meanwhile, qemu has a bug where '-smbios type=1,uuid=uuid' must parse as a valid uuid, but is otherwise ignored. The current qemu.git code base _only_ sets smbios uuid from the '-uuid uuid' argument. I've filed a bug against the qemu folks asking whether this is intended (in which case we have to modify libvirt to change the -uuid argument it outputs when smbios is specified), or an oversight (hopefully the latter, since it's still nice to correlate /var/log/libvirt/qemu/....log with the XML uuid even when that differs from the smbios uuid presented to the guest.)
Hmm, I thought the UUID we were passing in was a host UUID, but on closer inspection the type=1 table is definitely intended to carry the guest UUID. As such it doesn't make sense to populate that with anything other than the '<uuid>' from the guest XML. A host UUID should live elsewhere in the SMBIOS tables, likely in the type2 or type3 blocks Daniel
On 12/02/2010 05:47 AM, Daniel P. Berrange wrote:
On Wed, Dec 01, 2010 at 05:12:08PM -0700, Eric Blake wrote:
In testing <smbios mode='host'/>, I noticed a couple of problems. First, qemu rejected the command line we gave it (invalid UUID); ifixingthat showed that all other arguments were being given literal "" which then made the guest smbios differ from the host. Second, the qemu option -smbios type=1,family=string wasn't supported, which lead to a spurious difference between host and guest.
Meanwhile, qemu has a bug where '-smbios type=1,uuid=uuid' must parse as a valid uuid, but is otherwise ignored. The current qemu.git code base _only_ sets smbios uuid from the '-uuid uuid' argument. I've filed a bug against the qemu folks asking whether this is intended (in which case we have to modify libvirt to change the -uuid argument it outputs when smbios is specified), or an oversight (hopefully the latter, since it's still nice to correlate /var/log/libvirt/qemu/....log with the XML uuid even when that differs from the smbios uuid presented to the guest.)
Hmm, I thought the UUID we were passing in was a host UUID, but on closer inspection the type=1 table is definitely intended to carry the guest UUID. As such it doesn't make sense to populate that with anything other than the '<uuid>' from the guest XML. A host UUID should live elsewhere in the SMBIOS tables, likely in the type2 or type3 blocks
What does that mean to our XML? Should we reject XML files where both domain/uuid and domain/sysinfo/system/entry/uuid exist, but differ? Both elements are optional, so it's feasible to see a guest uuid in either location. Meanwhile, I'm waiting on resolution to https://bugzilla.redhat.com/show_bug.cgi?id=659122 to see if qemu will be patched to let us stick the host's uuid in block 2 (base board) or block 3 (chassis), in which case, we'll need to expand our XML to support that notion. I've also discovered that with current qemu, if both '-uuid uuid' and '-smbios type=1,uuid=uuid' are specified, then -uuid takes precedence; but either one of the two in isolation serves to set smbios block 1 with the guest's uuid. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Thu, Dec 02, 2010 at 10:36:59AM -0700, Eric Blake wrote:
On 12/02/2010 05:47 AM, Daniel P. Berrange wrote:
On Wed, Dec 01, 2010 at 05:12:08PM -0700, Eric Blake wrote:
In testing <smbios mode='host'/>, I noticed a couple of problems. First, qemu rejected the command line we gave it (invalid UUID); ifixingthat showed that all other arguments were being given literal "" which then made the guest smbios differ from the host. Second, the qemu option -smbios type=1,family=string wasn't supported, which lead to a spurious difference between host and guest.
Meanwhile, qemu has a bug where '-smbios type=1,uuid=uuid' must parse as a valid uuid, but is otherwise ignored. The current qemu.git code base _only_ sets smbios uuid from the '-uuid uuid' argument. I've filed a bug against the qemu folks asking whether this is intended (in which case we have to modify libvirt to change the -uuid argument it outputs when smbios is specified), or an oversight (hopefully the latter, since it's still nice to correlate /var/log/libvirt/qemu/....log with the XML uuid even when that differs from the smbios uuid presented to the guest.)
Hmm, I thought the UUID we were passing in was a host UUID, but on closer inspection the type=1 table is definitely intended to carry the guest UUID. As such it doesn't make sense to populate that with anything other than the '<uuid>' from the guest XML. A host UUID should live elsewhere in the SMBIOS tables, likely in the type2 or type3 blocks
What does that mean to our XML? Should we reject XML files where both domain/uuid and domain/sysinfo/system/entry/uuid exist, but differ? Both elements are optional, so it's feasible to see a guest uuid in either location. Meanwhile, I'm waiting on resolution to https://bugzilla.redhat.com/show_bug.cgi?id=659122 to see if qemu will be patched to let us stick the host's uuid in block 2 (base board) or block 3 (chassis), in which case, we'll need to expand our XML to support that notion.
As I commented on the BZ use OEM strings type 11 smbios table to pass anything you want into a guest.
I've also discovered that with current qemu, if both '-uuid uuid' and '-smbios type=1,uuid=uuid' are specified, then -uuid takes precedence; but either one of the two in isolation serves to set smbios block 1 with the guest's uuid.
I am surprised that libvirt still uses -uuid. -- Gleb.
On Thu, Dec 02, 2010 at 09:44:47PM +0200, Gleb Natapov wrote:
On Thu, Dec 02, 2010 at 10:36:59AM -0700, Eric Blake wrote:
On 12/02/2010 05:47 AM, Daniel P. Berrange wrote:
On Wed, Dec 01, 2010 at 05:12:08PM -0700, Eric Blake wrote:
In testing <smbios mode='host'/>, I noticed a couple of problems. First, qemu rejected the command line we gave it (invalid UUID); ifixingthat showed that all other arguments were being given literal "" which then made the guest smbios differ from the host. Second, the qemu option -smbios type=1,family=string wasn't supported, which lead to a spurious difference between host and guest.
Meanwhile, qemu has a bug where '-smbios type=1,uuid=uuid' must parse as a valid uuid, but is otherwise ignored. The current qemu.git code base _only_ sets smbios uuid from the '-uuid uuid' argument. I've filed a bug against the qemu folks asking whether this is intended (in which case we have to modify libvirt to change the -uuid argument it outputs when smbios is specified), or an oversight (hopefully the latter, since it's still nice to correlate /var/log/libvirt/qemu/....log with the XML uuid even when that differs from the smbios uuid presented to the guest.)
Hmm, I thought the UUID we were passing in was a host UUID, but on closer inspection the type=1 table is definitely intended to carry the guest UUID. As such it doesn't make sense to populate that with anything other than the '<uuid>' from the guest XML. A host UUID should live elsewhere in the SMBIOS tables, likely in the type2 or type3 blocks
What does that mean to our XML? Should we reject XML files where both domain/uuid and domain/sysinfo/system/entry/uuid exist, but differ? Both elements are optional, so it's feasible to see a guest uuid in either location. Meanwhile, I'm waiting on resolution to https://bugzilla.redhat.com/show_bug.cgi?id=659122 to see if qemu will be patched to let us stick the host's uuid in block 2 (base board) or block 3 (chassis), in which case, we'll need to expand our XML to support that notion.
As I commented on the BZ use OEM strings type 11 smbios table to pass anything you want into a guest.
I've also discovered that with current qemu, if both '-uuid uuid' and '-smbios type=1,uuid=uuid' are specified, then -uuid takes precedence; but either one of the two in isolation serves to set smbios block 1 with the guest's uuid.
I am surprised that libvirt still uses -uuid.
SMBIOS tables are only setup on x86 QEMU binaries, doing fullvirt. On non-x86, or QEMU used for Xen paravirt, the -uuid arg value would be used in other ways unrelated to SMBIOS. Thus replacing -uuid $UUID with -smbiios type=1,uuid=$UUID would be wrong. Regards, Daniel
On Thu, Dec 02, 2010 at 07:56:03PM +0000, Daniel P. Berrange wrote:
On Thu, Dec 02, 2010 at 09:44:47PM +0200, Gleb Natapov wrote:
On Thu, Dec 02, 2010 at 10:36:59AM -0700, Eric Blake wrote:
On 12/02/2010 05:47 AM, Daniel P. Berrange wrote:
On Wed, Dec 01, 2010 at 05:12:08PM -0700, Eric Blake wrote:
In testing <smbios mode='host'/>, I noticed a couple of problems. First, qemu rejected the command line we gave it (invalid UUID); ifixingthat showed that all other arguments were being given literal "" which then made the guest smbios differ from the host. Second, the qemu option -smbios type=1,family=string wasn't supported, which lead to a spurious difference between host and guest.
Meanwhile, qemu has a bug where '-smbios type=1,uuid=uuid' must parse as a valid uuid, but is otherwise ignored. The current qemu.git code base _only_ sets smbios uuid from the '-uuid uuid' argument. I've filed a bug against the qemu folks asking whether this is intended (in which case we have to modify libvirt to change the -uuid argument it outputs when smbios is specified), or an oversight (hopefully the latter, since it's still nice to correlate /var/log/libvirt/qemu/....log with the XML uuid even when that differs from the smbios uuid presented to the guest.)
Hmm, I thought the UUID we were passing in was a host UUID, but on closer inspection the type=1 table is definitely intended to carry the guest UUID. As such it doesn't make sense to populate that with anything other than the '<uuid>' from the guest XML. A host UUID should live elsewhere in the SMBIOS tables, likely in the type2 or type3 blocks
What does that mean to our XML? Should we reject XML files where both domain/uuid and domain/sysinfo/system/entry/uuid exist, but differ? Both elements are optional, so it's feasible to see a guest uuid in either location. Meanwhile, I'm waiting on resolution to https://bugzilla.redhat.com/show_bug.cgi?id=659122 to see if qemu will be patched to let us stick the host's uuid in block 2 (base board) or block 3 (chassis), in which case, we'll need to expand our XML to support that notion.
As I commented on the BZ use OEM strings type 11 smbios table to pass anything you want into a guest.
I've also discovered that with current qemu, if both '-uuid uuid' and '-smbios type=1,uuid=uuid' are specified, then -uuid takes precedence; but either one of the two in isolation serves to set smbios block 1 with the guest's uuid.
I am surprised that libvirt still uses -uuid.
SMBIOS tables are only setup on x86 QEMU binaries, doing fullvirt. On non-x86, or QEMU used for Xen paravirt, the -uuid arg value would be used in other ways unrelated to SMBIOS. Thus replacing -uuid $UUID with -smbiios type=1,uuid=$UUID would be wrong.
What non-x86 platforms libvirt supports? With Xen use -uuid or whatever they support. With KVM use only smbios type=1,uuid=$UUID. There is not valid reason that I see to use both. But if you use both of them for some strange reason please make sure you provide the same value for both. -- Gleb.
On Thu, Dec 02, 2010 at 10:04:59PM +0200, Gleb Natapov wrote:
On Thu, Dec 02, 2010 at 07:56:03PM +0000, Daniel P. Berrange wrote:
On Thu, Dec 02, 2010 at 09:44:47PM +0200, Gleb Natapov wrote:
On Thu, Dec 02, 2010 at 10:36:59AM -0700, Eric Blake wrote:
On 12/02/2010 05:47 AM, Daniel P. Berrange wrote:
On Wed, Dec 01, 2010 at 05:12:08PM -0700, Eric Blake wrote:
In testing <smbios mode='host'/>, I noticed a couple of problems. First, qemu rejected the command line we gave it (invalid UUID); ifixingthat showed that all other arguments were being given literal "" which then made the guest smbios differ from the host. Second, the qemu option -smbios type=1,family=string wasn't supported, which lead to a spurious difference between host and guest.
Meanwhile, qemu has a bug where '-smbios type=1,uuid=uuid' must parse as a valid uuid, but is otherwise ignored. The current qemu.git code base _only_ sets smbios uuid from the '-uuid uuid' argument. I've filed a bug against the qemu folks asking whether this is intended (in which case we have to modify libvirt to change the -uuid argument it outputs when smbios is specified), or an oversight (hopefully the latter, since it's still nice to correlate /var/log/libvirt/qemu/....log with the XML uuid even when that differs from the smbios uuid presented to the guest.)
Hmm, I thought the UUID we were passing in was a host UUID, but on closer inspection the type=1 table is definitely intended to carry the guest UUID. As such it doesn't make sense to populate that with anything other than the '<uuid>' from the guest XML. A host UUID should live elsewhere in the SMBIOS tables, likely in the type2 or type3 blocks
What does that mean to our XML? Should we reject XML files where both domain/uuid and domain/sysinfo/system/entry/uuid exist, but differ? Both elements are optional, so it's feasible to see a guest uuid in either location. Meanwhile, I'm waiting on resolution to https://bugzilla.redhat.com/show_bug.cgi?id=659122 to see if qemu will be patched to let us stick the host's uuid in block 2 (base board) or block 3 (chassis), in which case, we'll need to expand our XML to support that notion.
As I commented on the BZ use OEM strings type 11 smbios table to pass anything you want into a guest.
I've also discovered that with current qemu, if both '-uuid uuid' and '-smbios type=1,uuid=uuid' are specified, then -uuid takes precedence; but either one of the two in isolation serves to set smbios block 1 with the guest's uuid.
I am surprised that libvirt still uses -uuid.
SMBIOS tables are only setup on x86 QEMU binaries, doing fullvirt. On non-x86, or QEMU used for Xen paravirt, the -uuid arg value would be used in other ways unrelated to SMBIOS. Thus replacing -uuid $UUID with -smbiios type=1,uuid=$UUID would be wrong.
What non-x86 platforms libvirt supports? With Xen use -uuid or whatever they support. With KVM use only smbios type=1,uuid=$UUID. There is not valid reason that I see to use both. But if you use both of them for some strange reason please make sure you provide the same value for both.
libvirt aims to support any and all QEMU target architectures not merely x86. There's no benefit to using -smbios type=1,uuid=$UUID with KVM, over -uuid $UUID. Changing it only for KVM would needlessly complicate the code. Daniel
On Thu, Dec 02, 2010 at 08:12:23PM +0000, Daniel P. Berrange wrote:
On Thu, Dec 02, 2010 at 10:04:59PM +0200, Gleb Natapov wrote:
On Thu, Dec 02, 2010 at 07:56:03PM +0000, Daniel P. Berrange wrote:
On Thu, Dec 02, 2010 at 09:44:47PM +0200, Gleb Natapov wrote:
On Thu, Dec 02, 2010 at 10:36:59AM -0700, Eric Blake wrote:
On 12/02/2010 05:47 AM, Daniel P. Berrange wrote:
On Wed, Dec 01, 2010 at 05:12:08PM -0700, Eric Blake wrote: > In testing <smbios mode='host'/>, I noticed a couple of problems. > First, qemu rejected the command line we gave it (invalid UUID); > ifixingthat showed that all other arguments were being given literal > "" which then made the guest smbios differ from the host. Second, > the qemu option -smbios type=1,family=string wasn't supported, which > lead to a spurious difference between host and guest. > > Meanwhile, qemu has a bug where '-smbios type=1,uuid=uuid' must parse > as a valid uuid, but is otherwise ignored. The current qemu.git code > base _only_ sets smbios uuid from the '-uuid uuid' argument. I've > filed a bug against the qemu folks asking whether this is intended (in > which case we have to modify libvirt to change the -uuid argument it > outputs when smbios is specified), or an oversight (hopefully the > latter, since it's still nice to correlate > /var/log/libvirt/qemu/....log with the XML uuid even when that differs > from the smbios uuid presented to the guest.)
Hmm, I thought the UUID we were passing in was a host UUID, but on closer inspection the type=1 table is definitely intended to carry the guest UUID. As such it doesn't make sense to populate that with anything other than the '<uuid>' from the guest XML. A host UUID should live elsewhere in the SMBIOS tables, likely in the type2 or type3 blocks
What does that mean to our XML? Should we reject XML files where both domain/uuid and domain/sysinfo/system/entry/uuid exist, but differ? Both elements are optional, so it's feasible to see a guest uuid in either location. Meanwhile, I'm waiting on resolution to https://bugzilla.redhat.com/show_bug.cgi?id=659122 to see if qemu will be patched to let us stick the host's uuid in block 2 (base board) or block 3 (chassis), in which case, we'll need to expand our XML to support that notion.
As I commented on the BZ use OEM strings type 11 smbios table to pass anything you want into a guest.
I've also discovered that with current qemu, if both '-uuid uuid' and '-smbios type=1,uuid=uuid' are specified, then -uuid takes precedence; but either one of the two in isolation serves to set smbios block 1 with the guest's uuid.
I am surprised that libvirt still uses -uuid.
SMBIOS tables are only setup on x86 QEMU binaries, doing fullvirt. On non-x86, or QEMU used for Xen paravirt, the -uuid arg value would be used in other ways unrelated to SMBIOS. Thus replacing -uuid $UUID with -smbiios type=1,uuid=$UUID would be wrong.
What non-x86 platforms libvirt supports? With Xen use -uuid or whatever they support. With KVM use only smbios type=1,uuid=$UUID. There is not valid reason that I see to use both. But if you use both of them for some strange reason please make sure you provide the same value for both.
libvirt aims to support any and all QEMU target architectures not merely x86. There's no benefit to using -smbios type=1,uuid=$UUID with KVM, over -uuid $UUID. So why do you use them both? Even more interesting question why do you use them both with different $UUID? If you do, do not complain that there should be some kind of order between them.
Changing it only for KVM would needlessly complicate the code.
Doing things correctly is more important that simple code. And this is not related to KVM at all. You cannot expect qemu-sparc to work with qemu-x86 command line, so if you aim to support any and all qemu targets you should know how to build correct parameter list for any and all of them. -- Gleb.
On 12/03/2010 02:50 AM, Gleb Natapov wrote:
I am surprised that libvirt still uses -uuid.
Why? It still works, and it's backwards compatible even to older qemu where -smbios wasn't supported. It's actually harder to omit -uuid and use only -smbios, after first guaranteeing that -smbios is supported, than it is blindly use -uuid. And since -uuid may have more implications than just the -smbios table, whereas -smbios should not affect anything except the smbios table, it makes sense to continue to use -uuid for any of those other implications.
SMBIOS tables are only setup on x86 QEMU binaries, doing fullvirt. On non-x86, or QEMU used for Xen paravirt, the -uuid arg value would be used in other ways unrelated to SMBIOS. Thus replacing -uuid $UUID with -smbiios type=1,uuid=$UUID would be wrong.
What non-x86 platforms libvirt supports? With Xen use -uuid or whatever they support. With KVM use only smbios type=1,uuid=$UUID. There is not valid reason that I see to use both. But if you use both of them for some strange reason please make sure you provide the same value for both.
libvirt aims to support any and all QEMU target architectures not merely x86. There's no benefit to using -smbios type=1,uuid=$UUID with KVM, over -uuid $UUID. So why do you use them both? Even more interesting question why do you use them both with different $UUID? If you do, do not complain that there should be some kind of order between them.
There's the real question. We use both as of libvirt 0.8.6 because we added the new XML for <sysinfo type='smbios'>...</sysinfo><os><smbios mode='sysinfo'/></os>, where <sysinfo> provides the argument to pass to qemu's -smbios argument. However, I agree with your assessment that -uuid and -smbios type=1,uuid= should never differ, so I've proposed an updated patch series to guarantee that behavior (particularly patch 4/5): https://www.redhat.com/archives/libvir-list/2010-December/msg00237.html As for whether it is useful to pass the host UUID to the client through smbios, I agree with you that 1. it is still up in the air as to whether that is a decent design, or whether something better should be dreamed up, and 2. it would have to be done through smbios block 11 (OEM strings) and NOT by changing the guest's uuid (whether that uuid was supplied by <uuid>, by <sysinfo>, or identically in both locations, from the libvirt XML). But both of those points are part of the bigger picture, and shouldn't affect whether my patch series is accepted for fixing up current bugs in libvirt smbios handling (although it may imply future libvirt patches to start creating an appropriate binary file for use with -smbios binary=file for passing the host uuid as an oem string). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Fri, Dec 03, 2010 at 03:15:11PM -0700, Eric Blake wrote:
On 12/03/2010 02:50 AM, Gleb Natapov wrote:
I am surprised that libvirt still uses -uuid.
Why? It still works, and it's backwards compatible even to older qemu where -smbios wasn't supported. It's actually harder to omit -uuid and use only -smbios, after first guaranteeing that -smbios is supported, than it is blindly use -uuid. And since -uuid may have more implications than just the -smbios table, whereas -smbios should not affect anything except the smbios table, it makes sense to continue to use -uuid for any of those other implications.
There is not any additional implication using -uuid. In fact -uuid and -smbios table1,uuid= are completely identical from qemu's code point of view since both of them set the same global variable qemu_uuid. Currently there is not problem using both of them (with the same uuid value of course) and I do not think this will change in the near feature, so fixing libvirt to use only -smbios may be not the most urgent bug to fix. -- Gleb.
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Gleb Natapov