On Fri, Dec 03, 2010 at 02:56:14PM -0700, Eric Blake wrote: Hum, I was afraid that QEmu parsing would fail in case of spaces if there is no quote, but if you checked this, sure ! ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

* 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 08ebefb..3dd7aa3 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1875,6 +1875,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 9516427..3dfd4ee 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3625,6 +3625,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); @@ -6425,7 +6427,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, @@ -6451,6 +6454,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 8985241..9a9d924 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -3651,7 +3651,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"); @@ -3675,6 +3676,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.3.2

* 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. --- 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 3dd7aa3..811d559 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1881,7 +1881,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.3.2

* src/conf/domain_conf.c (virDomainDefParseXML): Prefer sysinfo uuid over generating one, and if both uuids are present, require them to be identical. * src/qemu/qemu_conf.c (qemuBuildSmbiosSystemStr): Allow skipping the uuid. (qemudBuildCommandLine): Adjust caller; <smbios mode=host/> must not use host uuid in place of guest uuid. --- The qemu developers pointed out that '-uuid xyz' and '-smbios type=1,uuid=xyz' both have the same effect. They argued that -uuid is deprecated, but it's easier for libvirt to always supply it. Meanwhile, they were clear that if both values were supplied, the two uuids had better be identical (current qemu favors -uuid over -smbios, but it's not guaranteed to stay that way in the future). src/conf/domain_conf.c | 22 +++++++++++++++++++++- src/qemu/qemu_conf.c | 18 +++++++++++------- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3dfd4ee..9c54a59 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4498,6 +4498,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, long id = -1; virDomainDefPtr def; unsigned long count; + bool uuid_generated = false; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -4529,7 +4530,9 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto error; } - /* Extract domain uuid */ + /* Extract domain uuid. If both uuid and sysinfo/system/entry/uuid + * exist, they must match; and if only the latter exists, it can + * also serve as the uuid. */ tmp = virXPathString("string(./uuid[1])", ctxt); if (!tmp) { if (virUUIDGenerate(def->uuid)) { @@ -4537,6 +4540,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, "%s", _("Failed to generate UUID")); goto error; } + uuid_generated = true; } else { if (virUUIDParse(tmp, def->uuid) < 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -5279,6 +5283,22 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, 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) { + virDomainReportError(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) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("UUID mismatch between <uuid> and " + "<sysinfo>")); + goto error; + } + } } tmp = virXPathString("string(./os/smbios/@mode)", ctxt); if (tmp) { diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 9a9d924..9ca1dad 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -3645,15 +3645,15 @@ error: return(NULL); } -static char *qemuBuildSmbiosSystemStr(virSysinfoDefPtr def) +static char *qemuBuildSmbiosSystemStr(virSysinfoDefPtr def, bool skip_uuid) { virBuffer buf = VIR_BUFFER_INITIALIZER; 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_family == NULL)) - return(NULL); + (def->system_product == NULL) && (def->system_version == NULL) && + (def->system_serial == NULL) && (def->system_family == NULL) && + (def->system_uuid == NULL || skip_uuid)) + return NULL; virBufferAddLit(&buf, "type=1"); @@ -3671,7 +3671,7 @@ static char *qemuBuildSmbiosSystemStr(virSysinfoDefPtr def) if (def->system_serial) virBufferVSprintf(&buf, ",serial=%s", def->system_serial); /* 1:UUID */ - if (def->system_uuid) + if (def->system_uuid && !skip_uuid) virBufferVSprintf(&buf, ",uuid=%s", def->system_uuid); /* 1:SKU Number */ if (def->system_sku) @@ -4173,6 +4173,7 @@ qemudBuildCommandLine(virConnectPtr conn, if ((def->os.smbios_mode != VIR_DOMAIN_SMBIOS_NONE) && (def->os.smbios_mode != VIR_DOMAIN_SMBIOS_EMULATE)) { virSysinfoDefPtr source = NULL; + bool skip_uuid = false; if (!(qemuCmdFlags & QEMUD_CMD_FLAG_SMBIOS_TYPE)) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -4189,6 +4190,8 @@ qemudBuildCommandLine(virConnectPtr conn, goto error; } source = driver->hostsysinfo; + /* Host and guest uuid must differ, by definition of UUID. */ + skip_uuid = true; } else if (def->os.smbios_mode == VIR_DOMAIN_SMBIOS_SYSINFO) { if (def->sysinfo == NULL) { qemuReportError(VIR_ERR_XML_ERROR, @@ -4197,6 +4200,7 @@ qemudBuildCommandLine(virConnectPtr conn, goto error; } source = def->sysinfo; + /* domain_conf guaranteed that system_uuid matches guest uuid. */ } if (source != NULL) { char *smbioscmd; @@ -4206,7 +4210,7 @@ qemudBuildCommandLine(virConnectPtr conn, virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); VIR_FREE(smbioscmd); } - smbioscmd = qemuBuildSmbiosSystemStr(source); + smbioscmd = qemuBuildSmbiosSystemStr(source, skip_uuid); if (smbioscmd != NULL) { virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); VIR_FREE(smbioscmd); -- 1.7.3.2

* src/util/sysinfo.c (virSysinfoRead): Use virCommand instead of virExec. --- This patch assumes option 2 of my patch to virCommand guarantees on outbuf when the child process had no output (it needs a slight tweak to avoid a NULL dereference if we go with option 1 for virCommand). src/util/sysinfo.c | 50 ++++++++++---------------------------------------- 1 files changed, 10 insertions(+), 40 deletions(-) diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c index cf41773..f44b112 100644 --- a/src/util/sysinfo.c +++ b/src/util/sysinfo.c @@ -36,6 +36,7 @@ #include "conf/domain_conf.h" #include "logging.h" #include "memory.h" +#include "command.h" #define VIR_FROM_THIS VIR_FROM_SYSINFO @@ -94,25 +95,24 @@ virSysinfoRead(void) { virSysinfoDefPtr virSysinfoRead(void) { char *path, *cur, *eol, *base; - int pid, outfd = -1, errfd = -1; virSysinfoDefPtr ret = NULL; const char *argv[] = { NULL, "-q", "-t", "0,1", NULL }; - int res, waitret, exitstatus; + int res; char *outbuf = NULL; - char *errbuf = NULL; + virCommandPtr cmd; path = virFindFileInPath(SYSINFO_SMBIOS_DECODER); if (path == NULL) { virSmbiosReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to find path for %s binary"), SYSINFO_SMBIOS_DECODER); - return(NULL); + return NULL; } - argv[0] = path; - res = virExec(argv, NULL, NULL, &pid, -1, &outfd, &errfd, - VIR_EXEC_NONE | VIR_EXEC_NONBLOCK); - if (res < 0) { + argv[0] = path; + cmd = virCommandNewArgs(argv); + virCommandSetOutputBuffer(cmd, &outbuf); + if (virCommandRun(cmd, NULL) < 0) { virSmbiosReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to execute command %s"), path); @@ -120,34 +120,6 @@ virSysinfoRead(void) { goto cleanup; } - /* - * we are interested in the output, capture it and errors too - */ - if (virPipeReadUntilEOF(outfd, errfd, &outbuf, &errbuf) < 0) { - virReportSystemError(errno, _("cannot wait for '%s'"), path); - while (waitpid(pid, &exitstatus, 0) == -1 && errno == EINTR) - ; - goto cleanup; - } - - if (outbuf) - VIR_DEBUG("Command stdout: %s", outbuf); - if (errbuf) - VIR_DEBUG("Command stderr: %s", errbuf); - - while ((waitret = waitpid(pid, &exitstatus, 0) == -1) && - (errno == EINTR)); - if (waitret == -1) { - virReportSystemError(errno, _("Failed to wait for '%s'"), path); - goto cleanup; - } - if (exitstatus != 0) { - virSmbiosReportError(VIR_ERR_INTERNAL_ERROR, - _("command %s failed with error code %d:%s"), - path, exitstatus, errbuf); - goto cleanup; - } - if (VIR_ALLOC(ret) < 0) goto no_memory; @@ -227,15 +199,13 @@ virSysinfoRead(void) { cleanup: VIR_FREE(outbuf); - VIR_FREE(errbuf); - VIR_FREE(path); + virCommandFree(cmd); - return(ret); + return ret; no_memory: virReportOOMError(); - virSysinfoDefFree(ret); ret = NULL; goto cleanup; -- 1.7.3.2

* src/util/sysinfo.c: Indentation and () fixups. --- No semantic change; noticed this while rebasing 5/5 of previous series (1-4 of previous series are now pushed). src/util/sysinfo.c | 37 ++++++++++++++++++------------------- 1 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c index cf41773..a02bd57 100644 --- a/src/util/sysinfo.c +++ b/src/util/sysinfo.c @@ -39,7 +39,7 @@ #define VIR_FROM_THIS VIR_FROM_SYSINFO -#define virSmbiosReportError(code, ...) \ +#define virSmbiosReportError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_SYSINFO, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) @@ -88,7 +88,7 @@ virSysinfoRead(void) { */ virReportSystemError(ENOSYS, "%s", _("Host sysinfo extraction not supported on this platform")); - return(NULL); + return NULL; } #else virSysinfoDefPtr @@ -104,9 +104,9 @@ virSysinfoRead(void) { path = virFindFileInPath(SYSINFO_SMBIOS_DECODER); if (path == NULL) { virSmbiosReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to find path for %s binary"), + _("Failed to find path for %s binary"), SYSINFO_SMBIOS_DECODER); - return(NULL); + return NULL; } argv[0] = path; @@ -114,7 +114,7 @@ virSysinfoRead(void) { VIR_EXEC_NONE | VIR_EXEC_NONBLOCK); if (res < 0) { virSmbiosReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to execute command %s"), + _("Failed to execute command %s"), path); res = 1; goto cleanup; @@ -143,7 +143,7 @@ virSysinfoRead(void) { } if (exitstatus != 0) { virSmbiosReportError(VIR_ERR_INTERNAL_ERROR, - _("command %s failed with error code %d:%s"), + _("command %s failed with error code %d:%s"), path, exitstatus, errbuf); goto cleanup; } @@ -159,25 +159,25 @@ virSysinfoRead(void) { cur += 8; eol = strchr(cur, '\n'); if ((eol) && ((ret->bios_vendor = strndup(cur, eol - cur)) == NULL)) - goto no_memory; + goto no_memory; } if ((cur = strstr(base, "Version: ")) != NULL) { cur += 9; eol = strchr(cur, '\n'); if ((eol) && ((ret->bios_version = strndup(cur, eol - cur)) == NULL)) - goto no_memory; + goto no_memory; } if ((cur = strstr(base, "Release Date: ")) != NULL) { cur += 14; eol = strchr(cur, '\n'); if ((eol) && ((ret->bios_date = strndup(cur, eol - cur)) == NULL)) - goto no_memory; + goto no_memory; } if ((cur = strstr(base, "BIOS Revision: ")) != NULL) { cur += 15; eol = strchr(cur, '\n'); if ((eol) && ((ret->bios_release = strndup(cur, eol - cur)) == NULL)) - goto no_memory; + goto no_memory; } if ((base = strstr(outbuf, "System Information")) == NULL) goto cleanup; @@ -186,43 +186,43 @@ virSysinfoRead(void) { eol = strchr(cur, '\n'); if ((eol) && ((ret->system_manufacturer = strndup(cur, eol - cur)) == NULL)) - goto no_memory; + goto no_memory; } if ((cur = strstr(base, "Product Name: ")) != NULL) { cur += 14; eol = strchr(cur, '\n'); if ((eol) && ((ret->system_product = strndup(cur, eol - cur)) == NULL)) - goto no_memory; + goto no_memory; } if ((cur = strstr(base, "Version: ")) != NULL) { cur += 9; eol = strchr(cur, '\n'); if ((eol) && ((ret->system_version = strndup(cur, eol - cur)) == NULL)) - goto no_memory; + goto no_memory; } if ((cur = strstr(base, "Serial Number: ")) != NULL) { cur += 15; eol = strchr(cur, '\n'); if ((eol) && ((ret->system_serial = strndup(cur, eol - cur)) == NULL)) - goto no_memory; + goto no_memory; } if ((cur = strstr(base, "UUID: ")) != NULL) { cur += 6; eol = strchr(cur, '\n'); if ((eol) && ((ret->system_uuid = strndup(cur, eol - cur)) == NULL)) - goto no_memory; + goto no_memory; } if ((cur = strstr(base, "SKU Number: ")) != NULL) { cur += 12; eol = strchr(cur, '\n'); if ((eol) && ((ret->system_sku = strndup(cur, eol - cur)) == NULL)) - goto no_memory; + 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; + goto no_memory; } cleanup: @@ -230,12 +230,11 @@ cleanup: VIR_FREE(errbuf); VIR_FREE(path); - return(ret); + return ret; no_memory: virReportOOMError(); - virSysinfoDefFree(ret); ret = NULL; goto cleanup; -- 1.7.3.2

* src/util/sysinfo.c (virSysinfoRead): Use virCommand instead of virExec. --- v2: remove unused variable, rebase on top of whitespace cleanups. This patch is awaiting review on these prerequisites: https://www.redhat.com/archives/libvir-list/2010-December/msg00318.html https://www.redhat.com/archives/libvir-list/2010-December/msg00321.html src/util/sysinfo.c | 45 ++++++--------------------------------------- 1 files changed, 6 insertions(+), 39 deletions(-) diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c index a02bd57..5b40a10 100644 --- a/src/util/sysinfo.c +++ b/src/util/sysinfo.c @@ -36,6 +36,7 @@ #include "conf/domain_conf.h" #include "logging.h" #include "memory.h" +#include "command.h" #define VIR_FROM_THIS VIR_FROM_SYSINFO @@ -94,12 +95,9 @@ virSysinfoRead(void) { virSysinfoDefPtr virSysinfoRead(void) { char *path, *cur, *eol, *base; - int pid, outfd = -1, errfd = -1; virSysinfoDefPtr ret = NULL; - const char *argv[] = { NULL, "-q", "-t", "0,1", NULL }; - int res, waitret, exitstatus; char *outbuf = NULL; - char *errbuf = NULL; + virCommandPtr cmd; path = virFindFileInPath(SYSINFO_SMBIOS_DECODER); if (path == NULL) { @@ -108,43 +106,13 @@ virSysinfoRead(void) { SYSINFO_SMBIOS_DECODER); return NULL; } - argv[0] = path; - res = virExec(argv, NULL, NULL, &pid, -1, &outfd, &errfd, - VIR_EXEC_NONE | VIR_EXEC_NONBLOCK); - if (res < 0) { + cmd = virCommandNewArgList(path, "-q", "-t", "0,1", NULL); + virCommandSetOutputBuffer(cmd, &outbuf); + if (virCommandRun(cmd, NULL) < 0) { virSmbiosReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to execute command %s"), path); - res = 1; - goto cleanup; - } - - /* - * we are interested in the output, capture it and errors too - */ - if (virPipeReadUntilEOF(outfd, errfd, &outbuf, &errbuf) < 0) { - virReportSystemError(errno, _("cannot wait for '%s'"), path); - while (waitpid(pid, &exitstatus, 0) == -1 && errno == EINTR) - ; - goto cleanup; - } - - if (outbuf) - VIR_DEBUG("Command stdout: %s", outbuf); - if (errbuf) - VIR_DEBUG("Command stderr: %s", errbuf); - - while ((waitret = waitpid(pid, &exitstatus, 0) == -1) && - (errno == EINTR)); - if (waitret == -1) { - virReportSystemError(errno, _("Failed to wait for '%s'"), path); - goto cleanup; - } - if (exitstatus != 0) { - virSmbiosReportError(VIR_ERR_INTERNAL_ERROR, - _("command %s failed with error code %d:%s"), - path, exitstatus, errbuf); goto cleanup; } @@ -227,8 +195,7 @@ virSysinfoRead(void) { cleanup: VIR_FREE(outbuf); - VIR_FREE(errbuf); - VIR_FREE(path); + virCommandFree(cmd); return ret; -- 1.7.3.2

On 12/13/2010 04:10 PM, Matthias Bolte wrote: allocated... the old code freed it as part of argv cleanups oh, you're right - the new code strdup's it, so I did leak it. I've restored the VIR_FREE(path). Thanks for the review, and pushed. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org