11:17 p.m.
Hi,
I propose to add hardware Processor/MemoryDevice information to virSysinfoRead() from
SMBIOS.
I want to get host machine's these infomations through virConnectGetSysinfo().
I think it's useful to managing many host machines,
because administrator can centrally manage the host machines which consists of
Processor/MemoryDevice parts.
This patch set adds DMI type4, 17 information(dmidecode -t4,17) to virSysinfoRead().
I hope this patch set adds v0.9.3, if possible.
Minoru Usui (3):
Cleanup virSysinfoRead() Separate BIOSInfo and SystemInfo part
from virSysinfoRead()
Add Processor Information to virSysinfoRead() from dmidecode type 4
Add Memory Device Information to virSysinfoRead() from dmidecode type
17
src/util/sysinfo.c | 608 +++++++++++++++++++++++++++++++++++++++++++++-------
src/util/sysinfo.h | 37 ++++
2 files changed, 565 insertions(+), 80 deletions(-)
--
Minoru Usui <usui(a)mxm.nes.nec.co.jp>
11:19 p.m.
New subject: [libvirt] [PATCH 1/3] Cleanup virSysinfoRead()
[Cleanup] Separate BIOSInfo and SystemInfo part from virSysinfoRead()
Signed-off-by: Minoru Usui <usui(a)mxm.nes.nec.co.jp>
---
src/util/sysinfo.c | 210 ++++++++++++++++++++++++++++++++--------------------
1 files changed, 130 insertions(+), 80 deletions(-)
diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
index 40ec2e3..53122f7 100644
--- a/src/util/sysinfo.c
+++ b/src/util/sysinfo.c
@@ -96,37 +96,10 @@ virSysinfoRead(void) {
#else /* !WIN32 */
-virSysinfoDefPtr
-virSysinfoRead(void) {
- char *path, *cur, *eol, *base;
- virSysinfoDefPtr ret = NULL;
- char *outbuf = 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;
- }
-
- cmd = virCommandNewArgList(path, "-q", "-t", "0,1",
NULL);
- VIR_FREE(path);
- virCommandSetOutputBuffer(cmd, &outbuf);
- if (virCommandRun(cmd, NULL) < 0) {
- virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to execute command %s"),
- path);
- goto cleanup;
- }
-
- if (VIR_ALLOC(ret) < 0)
- goto no_memory;
-
- ret->type = VIR_SYSINFO_SMBIOS;
-
- base = outbuf;
+static char *
+parseBIOSInfo(char *base, virSysinfoDefPtr ret)
+{
+ char *cur, *eol;
if ((cur = strstr(base, "Vendor: ")) != NULL) {
cur += 8;
@@ -152,8 +125,21 @@ virSysinfoRead(void) {
if ((eol) && ((ret->bios_release = strndup(cur, eol - cur)) == NULL))
goto no_memory;
}
- if ((base = strstr(outbuf, "System Information")) == NULL)
- goto cleanup;
+
+ return eol + 1;
+
+no_memory:
+ return NULL;
+}
+
+static char *
+parseSystemInfo(char *base, virSysinfoDefPtr ret)
+{
+ char *cur, *eol;
+
+ if ((base = strstr(base, "System Information")) == NULL)
+ return 0;
+
if ((cur = strstr(base, "Manufacturer: ")) != NULL) {
cur += 14;
eol = strchr(cur, '\n');
@@ -198,6 +184,50 @@ virSysinfoRead(void) {
goto no_memory;
}
+ return eol + 1;
+
+no_memory:
+ return NULL;
+}
+
+virSysinfoDefPtr
+virSysinfoRead(void) {
+ char *path, *base;
+ virSysinfoDefPtr ret = NULL;
+ char *outbuf = 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;
+ }
+
+ cmd = virCommandNewArgList(path, "-q", "-t",
"0,1,4,17", NULL);
+ VIR_FREE(path);
+ virCommandSetOutputBuffer(cmd, &outbuf);
+ if (virCommandRun(cmd, NULL) < 0) {
+ virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to execute command %s"),
+ path);
+ goto cleanup;
+ }
+
+ if (VIR_ALLOC(ret) < 0)
+ goto no_memory;
+
+ ret->type = VIR_SYSINFO_SMBIOS;
+
+ base = outbuf;
+
+ if ((base = parseBIOSInfo(base, ret)) == NULL)
+ goto no_memory;
+
+ if ((base = parseSystemInfo(base, ret)) == NULL)
+ goto no_memory;
+
cleanup:
VIR_FREE(outbuf);
virCommandFree(cmd);
@@ -213,108 +243,128 @@ no_memory:
}
#endif /* !WIN32 */
-/**
- * virSysinfoFormat:
- * @def: structure to convert to xml string
- * @prefix: string to prefix before each line of xml
- *
- * This returns the XML description of the sysinfo, or NULL after
- * generating an error message.
- */
-char *
-virSysinfoFormat(virSysinfoDefPtr def, const char *prefix)
+static void
+BIOSInfoFormat(virSysinfoDefPtr def, const char *prefix, virBufferPtr buf)
{
- const char *type = virSysinfoTypeToString(def->type);
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- size_t len = strlen(prefix);
-
- if (!type) {
- virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected sysinfo type model %d"),
- def->type);
- return NULL;
- }
+ int len = strlen(prefix);
- virBufferAsprintf(&buf, "%s<sysinfo type='%s'>\n",
prefix, type);
if ((def->bios_vendor != NULL) || (def->bios_version != NULL) ||
(def->bios_date != NULL) || (def->bios_release != NULL)) {
- virBufferAsprintf(&buf, "%s <bios>\n", prefix);
+ virBufferAsprintf(buf, "%s <bios>\n", prefix);
if (def->bios_vendor != NULL) {
- virBufferAdd(&buf, prefix, len);
- virBufferEscapeString(&buf,
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
" <entry
name='vendor'>%s</entry>\n",
def->bios_vendor);
}
if (def->bios_version != NULL) {
- virBufferAdd(&buf, prefix, len);
- virBufferEscapeString(&buf,
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
" <entry
name='version'>%s</entry>\n",
def->bios_version);
}
if (def->bios_date != NULL) {
- virBufferAdd(&buf, prefix, len);
- virBufferEscapeString(&buf,
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
" <entry
name='date'>%s</entry>\n",
def->bios_date);
}
if (def->bios_release != NULL) {
- virBufferAdd(&buf, prefix, len);
- virBufferEscapeString(&buf,
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
" <entry
name='release'>%s</entry>\n",
def->bios_release);
}
- virBufferAsprintf(&buf, "%s </bios>\n", prefix);
+ virBufferAsprintf(buf, "%s </bios>\n", prefix);
}
+
+ return;
+}
+
+static void
+SystemInfoFormat(virSysinfoDefPtr def, const char *prefix, virBufferPtr buf)
+{
+ int len = strlen(prefix);
+
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_family != NULL)) {
- virBufferAsprintf(&buf, "%s <system>\n", prefix);
+ virBufferAsprintf(buf, "%s <system>\n", prefix);
if (def->system_manufacturer != NULL) {
- virBufferAdd(&buf, prefix, len);
- virBufferEscapeString(&buf,
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
" <entry
name='manufacturer'>%s</entry>\n",
def->system_manufacturer);
}
if (def->system_product != NULL) {
- virBufferAdd(&buf, prefix, len);
- virBufferEscapeString(&buf,
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
" <entry
name='product'>%s</entry>\n",
def->system_product);
}
if (def->system_version != NULL) {
- virBufferAdd(&buf, prefix, len);
- virBufferEscapeString(&buf,
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
" <entry
name='version'>%s</entry>\n",
def->system_version);
}
if (def->system_serial != NULL) {
- virBufferAdd(&buf, prefix, len);
- virBufferEscapeString(&buf,
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
" <entry
name='serial'>%s</entry>\n",
def->system_serial);
}
if (def->system_uuid != NULL) {
- virBufferAdd(&buf, prefix, len);
- virBufferEscapeString(&buf,
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
" <entry
name='uuid'>%s</entry>\n",
def->system_uuid);
}
if (def->system_sku != NULL) {
- virBufferAdd(&buf, prefix, len);
- virBufferEscapeString(&buf,
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
" <entry
name='sku'>%s</entry>\n",
def->system_sku);
}
if (def->system_family != NULL) {
- virBufferAdd(&buf, prefix, len);
- virBufferEscapeString(&buf,
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
" <entry
name='family'>%s</entry>\n",
def->system_family);
}
- virBufferAsprintf(&buf, "%s </system>\n", prefix);
+ virBufferAsprintf(buf, "%s </system>\n", prefix);
+ }
+
+ return;
+}
+
+/**
+ * virSysinfoFormat:
+ * @def: structure to convert to xml string
+ * @prefix: string to prefix before each line of xml
+ *
+ * This returns the XML description of the sysinfo, or NULL after
+ * generating an error message.
+ */
+char *
+virSysinfoFormat(virSysinfoDefPtr def, const char *prefix)
+{
+ const char *type = virSysinfoTypeToString(def->type);
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+ if (!type) {
+ virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected sysinfo type model %d"),
+ def->type);
+ return NULL;
}
+ virBufferAsprintf(&buf, "%s<sysinfo type='%s'>\n",
prefix, type);
+
+ BIOSInfoFormat(def, prefix, &buf);
+ SystemInfoFormat(def, prefix, &buf);
+
virBufferAsprintf(&buf, "%s</sysinfo>\n", prefix);
return virBufferContentAndReset(&buf);
--
1.7.1
9:21 a.m.
New subject: [libvirt] [PATCH 1/3] Cleanup virSysinfoRead()
On Tue, Jun 21, 2011 at 01:19:17PM +0900, Minoru Usui wrote:
[Cleanup] Separate BIOSInfo and SystemInfo part from
virSysinfoRead()
Signed-off-by: Minoru Usui <usui(a)mxm.nes.nec.co.jp>
---
src/util/sysinfo.c | 210 ++++++++++++++++++++++++++++++++--------------------
1 files changed, 130 insertions(+), 80 deletions(-)
diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
index 40ec2e3..53122f7 100644
--- a/src/util/sysinfo.c
+++ b/src/util/sysinfo.c
@@ -96,37 +96,10 @@ virSysinfoRead(void) {
#else /* !WIN32 */
-virSysinfoDefPtr
-virSysinfoRead(void) {
- char *path, *cur, *eol, *base;
- virSysinfoDefPtr ret = NULL;
- char *outbuf = 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;
- }
-
- cmd = virCommandNewArgList(path, "-q", "-t", "0,1",
NULL);
- VIR_FREE(path);
- virCommandSetOutputBuffer(cmd, &outbuf);
- if (virCommandRun(cmd, NULL) < 0) {
- virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to execute command %s"),
- path);
- goto cleanup;
- }
-
- if (VIR_ALLOC(ret) < 0)
- goto no_memory;
-
- ret->type = VIR_SYSINFO_SMBIOS;
-
- base = outbuf;
+static char *
+parseBIOSInfo(char *base, virSysinfoDefPtr ret)
Can you make sure this method, and all other ones added
to this file have the "virSysinfo" prefix on their name
+static char *
+parseSystemInfo(char *base, virSysinfoDefPtr ret)
+{
+static void
+BIOSInfoFormat(virSysinfoDefPtr def, const char *prefix, virBufferPtr buf)
+static void
+SystemInfoFormat(virSysinfoDefPtr def, const char *prefix, virBufferPtr buf)
+{
+/**
+ * virSysinfoFormat:
+ * @def: structure to convert to xml string
+ * @prefix: string to prefix before each line of xml
+ *
+ * This returns the XML description of the sysinfo, or NULL after
+ * generating an error message.
+ */
+char *
+virSysinfoFormat(virSysinfoDefPtr def, const char *prefix)
+{
+ const char *type = virSysinfoTypeToString(def->type);
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+ if (!type) {
+ virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected sysinfo type model %d"),
+ def->type);
+ return NULL;
}
+ virBufferAsprintf(&buf, "%s<sysinfo type='%s'>\n",
prefix, type);
+
+ BIOSInfoFormat(def, prefix, &buf);
+ SystemInfoFormat(def, prefix, &buf);
+
virBufferAsprintf(&buf, "%s</sysinfo>\n", prefix);
return virBufferContentAndReset(&buf);
An existing bug here that needs fixing. There should be a call to
if (virBufferError(&buf)) {
virReportOOMError();
return NULL;
}
just before the virBufferContentAndReset() usage.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:06 p.m.
New subject: [libvirt] [PATCH 1/3] Cleanup virSysinfoRead()
Hi, Daniel
Thank you for your review.
On Fri, 24 Jun 2011 15:21:36 +0100
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Tue, Jun 21, 2011 at 01:19:17PM +0900, Minoru Usui wrote:
> [Cleanup] Separate BIOSInfo and SystemInfo part from virSysinfoRead()
>
>
> Signed-off-by: Minoru Usui <usui(a)mxm.nes.nec.co.jp>
> ---
> src/util/sysinfo.c | 210 ++++++++++++++++++++++++++++++++--------------------
> 1 files changed, 130 insertions(+), 80 deletions(-)
>
> diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
> index 40ec2e3..53122f7 100644
> --- a/src/util/sysinfo.c
> +++ b/src/util/sysinfo.c
> @@ -96,37 +96,10 @@ virSysinfoRead(void) {
>
> #else /* !WIN32 */
>
> -virSysinfoDefPtr
> -virSysinfoRead(void) {
> - char *path, *cur, *eol, *base;
> - virSysinfoDefPtr ret = NULL;
> - char *outbuf = 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;
> - }
> -
> - cmd = virCommandNewArgList(path, "-q", "-t",
"0,1", NULL);
> - VIR_FREE(path);
> - virCommandSetOutputBuffer(cmd, &outbuf);
> - if (virCommandRun(cmd, NULL) < 0) {
> - virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
> - _("Failed to execute command %s"),
> - path);
> - goto cleanup;
> - }
> -
> - if (VIR_ALLOC(ret) < 0)
> - goto no_memory;
> -
> - ret->type = VIR_SYSINFO_SMBIOS;
> -
> - base = outbuf;
> +static char *
> +parseBIOSInfo(char *base, virSysinfoDefPtr ret)
Can you make sure this method, and all other ones added
to this file have the "virSysinfo" prefix on their name
Because of static functions, I didn't add prefixes.
I will add prefixes to all functions.
> +static char *
> +parseSystemInfo(char *base, virSysinfoDefPtr ret)
> +{
> +static void
> +BIOSInfoFormat(virSysinfoDefPtr def, const char *prefix, virBufferPtr buf)
> +static void
> +SystemInfoFormat(virSysinfoDefPtr def, const char *prefix, virBufferPtr buf)
> +{
> +/**
> + * virSysinfoFormat:
> + * @def: structure to convert to xml string
> + * @prefix: string to prefix before each line of xml
> + *
> + * This returns the XML description of the sysinfo, or NULL after
> + * generating an error message.
> + */
> +char *
> +virSysinfoFormat(virSysinfoDefPtr def, const char *prefix)
> +{
> + const char *type = virSysinfoTypeToString(def->type);
> + virBuffer buf = VIR_BUFFER_INITIALIZER;
> +
> + if (!type) {
> + virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unexpected sysinfo type model %d"),
> + def->type);
> + return NULL;
> }
>
> + virBufferAsprintf(&buf, "%s<sysinfo type='%s'>\n",
prefix, type);
> +
> + BIOSInfoFormat(def, prefix, &buf);
> + SystemInfoFormat(def, prefix, &buf);
> +
> virBufferAsprintf(&buf, "%s</sysinfo>\n", prefix);
>
> return virBufferContentAndReset(&buf);
An existing bug here that needs fixing. There should be a call to
if (virBufferError(&buf)) {
virReportOOMError();
return NULL;
}
just before the virBufferContentAndReset() usage.
OK.
I'll fix it.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
Minoru Usui <usui(a)mxm.nes.nec.co.jp>
11:20 p.m.
New subject: [libvirt] [PATCH 2/3] Add Processor Information to virSysinfoRead() from dmidecode type 4
Add Processor Information to virSysinfoRead() from dmidecode type 4
Signed-off-by: Minoru Usui <usui(a)mxm.nes.nec.co.jp>
---
src/util/sysinfo.c | 207 +++++++++++++++++++++++++++++++++++++++++++++++++++-
src/util/sysinfo.h | 19 +++++
2 files changed, 225 insertions(+), 1 deletions(-)
diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
index 53122f7..a1eb92b 100644
--- a/src/util/sysinfo.c
+++ b/src/util/sysinfo.c
@@ -72,6 +72,9 @@ void virSysinfoDefFree(virSysinfoDefPtr def)
VIR_FREE(def->system_uuid);
VIR_FREE(def->system_sku);
VIR_FREE(def->system_family);
+
+ VIR_FREE(def->processor);
+
VIR_FREE(def);
}
@@ -190,6 +193,107 @@ no_memory:
return NULL;
}
+static char *
+parseProcessorInfo(char *base, virSysinfoDefPtr ret)
+{
+ char *cur, *eol, *tmp_base;
+ virProcessorinfoDefPtr processor;
+
+ while((tmp_base = strstr(base, "Processor Information")) != NULL) {
+ base = tmp_base;
+
+ if (VIR_EXPAND_N(ret->processor, ret->nprocessor, 1) < 0) {
+ goto no_memory;
+ }
+ processor = &ret->processor[ret->nprocessor - 1];
+
+ if ((cur = strstr(base, "Socket Designation: ")) != NULL) {
+ cur += 20;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((processor->processor_socket_destination = strndup(cur, eol - cur))
== NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Type: ")) != NULL) {
+ cur += 6;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((processor->processor_type = strndup(cur, eol - cur)) == NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Family: ")) != NULL) {
+ cur += 8;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((processor->processor_family = strndup(cur, eol - cur)) == NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Manufacturer: ")) != NULL) {
+ cur += 14;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((processor->processor_manufacturer = strndup(cur, eol - cur)) ==
NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Signature: ")) != NULL) {
+ cur += 11;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((processor->processor_signature = strndup(cur, eol - cur)) == NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Version: ")) != NULL) {
+ cur += 9;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((processor->processor_version = strndup(cur, eol - cur)) == NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "External Clock: ")) != NULL) {
+ cur += 16;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((processor->processor_external_clock = strndup(cur, eol - cur)) ==
NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Max Speed: ")) != NULL) {
+ cur += 11;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((processor->processor_max_speed = strndup(cur, eol - cur)) == NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Status: ")) != NULL) {
+ cur += 8;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((processor->processor_status = strndup(cur, eol - cur)) == NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Serial Number: ")) != NULL) {
+ cur += 15;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((processor->processor_serial_number = strndup(cur, eol - cur)) ==
NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Part Number: ")) != NULL) {
+ cur += 13;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((processor->processor_part_number = strndup(cur, eol - cur)) ==
NULL))
+ goto no_memory;
+ }
+
+ base = eol + 1;
+ }
+
+ return base;
+
+no_memory:
+ return NULL;
+}
+
virSysinfoDefPtr
virSysinfoRead(void) {
char *path, *base;
@@ -205,7 +309,7 @@ virSysinfoRead(void) {
return NULL;
}
- cmd = virCommandNewArgList(path, "-q", "-t",
"0,1,4,17", NULL);
+ cmd = virCommandNewArgList(path, "-q", "-t", "0,1,4",
NULL);
VIR_FREE(path);
virCommandSetOutputBuffer(cmd, &outbuf);
if (virCommandRun(cmd, NULL) < 0) {
@@ -228,6 +332,11 @@ virSysinfoRead(void) {
if ((base = parseSystemInfo(base, ret)) == NULL)
goto no_memory;
+ ret->nprocessor = 0;
+ ret->processor = NULL;
+ if ((base = parseProcessorInfo(base, ret)) == NULL)
+ goto no_memory;
+
cleanup:
VIR_FREE(outbuf);
virCommandFree(cmd);
@@ -339,6 +448,101 @@ SystemInfoFormat(virSysinfoDefPtr def, const char *prefix,
virBufferPtr buf)
return;
}
+static void
+ProcessorInfoFormat(virSysinfoDefPtr def, const char *prefix, virBufferPtr buf)
+{
+ int i;
+ int len = strlen(prefix);
+ virProcessorinfoDefPtr processor;
+
+ for (i = 0; i < def->nprocessor; i++) {
+ processor = &def->processor[i];
+
+ if ((processor->processor_socket_destination != NULL) ||
+ (processor->processor_type != NULL) ||
+ (processor->processor_family != NULL) ||
+ (processor->processor_manufacturer != NULL) ||
+ (processor->processor_signature != NULL) ||
+ (processor->processor_version != NULL) ||
+ (processor->processor_external_clock != NULL) ||
+ (processor->processor_max_speed != NULL) ||
+ (processor->processor_status != NULL) ||
+ (processor->processor_serial_number != NULL) ||
+ (processor->processor_part_number != NULL)) {
+ virBufferAsprintf(buf, "%s <processor>\n", prefix);
+ if (processor->processor_socket_destination != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='socket_destination'>%s</entry>\n",
+ processor->processor_socket_destination);
+ }
+ if (processor->processor_type != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='type'>%s</entry>\n",
+ processor->processor_type);
+ }
+ if (processor->processor_family != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='family'>%s</entry>\n",
+ processor->processor_family);
+ }
+ if (processor->processor_manufacturer != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='manufacturer'>%s</entry>\n",
+ processor->processor_manufacturer);
+ }
+ if (processor->processor_signature != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='signature'>%s</entry>\n",
+ processor->processor_signature);
+ }
+ if (processor->processor_version != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='version'>%s</entry>\n",
+ processor->processor_version);
+ }
+ if (processor->processor_external_clock != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='external_clock'>%s</entry>\n",
+ processor->processor_external_clock);
+ }
+ if (processor->processor_max_speed != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='max_speed'>%s</entry>\n",
+ processor->processor_max_speed);
+ }
+ if (processor->processor_status != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='status'>%s</entry>\n",
+ processor->processor_status);
+ }
+ if (processor->processor_serial_number != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='serial_number'>%s</entry>\n",
+ processor->processor_serial_number);
+ }
+ if (processor->processor_part_number != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='part_number'>%s</entry>\n",
+ processor->processor_part_number);
+ }
+ virBufferAsprintf(buf, "%s </processor>\n", prefix);
+ }
+ }
+
+ return;
+}
+
/**
* virSysinfoFormat:
* @def: structure to convert to xml string
@@ -364,6 +568,7 @@ virSysinfoFormat(virSysinfoDefPtr def, const char *prefix)
BIOSInfoFormat(def, prefix, &buf);
SystemInfoFormat(def, prefix, &buf);
+ ProcessorInfoFormat(def, prefix, &buf);
virBufferAsprintf(&buf, "%s</sysinfo>\n", prefix);
diff --git a/src/util/sysinfo.h b/src/util/sysinfo.h
index f69b76c..f098e9d 100644
--- a/src/util/sysinfo.h
+++ b/src/util/sysinfo.h
@@ -33,6 +33,22 @@ enum virSysinfoType {
VIR_SYSINFO_LAST
};
+typedef struct _virProcessorinfoDef virProcessorinfoDef;
+typedef virProcessorinfoDef *virProcessorinfoDefPtr;
+struct _virProcessorinfoDef {
+ char *processor_socket_destination;
+ char *processor_type;
+ char *processor_family;
+ char *processor_manufacturer;
+ char *processor_signature;
+ char *processor_version;
+ char *processor_external_clock;
+ char *processor_max_speed;
+ char *processor_status;
+ char *processor_serial_number;
+ char *processor_part_number;
+};
+
typedef struct _virSysinfoDef virSysinfoDef;
typedef virSysinfoDef *virSysinfoDefPtr;
struct _virSysinfoDef {
@@ -50,6 +66,9 @@ struct _virSysinfoDef {
char *system_uuid;
char *system_sku;
char *system_family;
+
+ size_t nprocessor;
+ virProcessorinfoDefPtr processor;
};
virSysinfoDefPtr virSysinfoRead(void);
--
1.7.1
9:24 a.m.
New subject: [libvirt] [PATCH 2/3] Add Processor Information to virSysinfoRead() from dmidecode type 4
On Tue, Jun 21, 2011 at 01:20:30PM +0900, Minoru Usui wrote:
Add Processor Information to virSysinfoRead() from dmidecode type 4
Signed-off-by: Minoru Usui <usui(a)mxm.nes.nec.co.jp>
---
src/util/sysinfo.c | 207 +++++++++++++++++++++++++++++++++++++++++++++++++++-
src/util/sysinfo.h | 19 +++++
2 files changed, 225 insertions(+), 1 deletions(-)
diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
index 53122f7..a1eb92b 100644
--- a/src/util/sysinfo.c
+++ b/src/util/sysinfo.c
@@ -72,6 +72,9 @@ void virSysinfoDefFree(virSysinfoDefPtr def)
VIR_FREE(def->system_uuid);
VIR_FREE(def->system_sku);
VIR_FREE(def->system_family);
+
+ VIR_FREE(def->processor);
This frees the 'virProcessinfoDef' instance, but what
about all the allocated strings inside it ?
+
VIR_FREE(def);
}
@@ -190,6 +193,107 @@ no_memory:
return NULL;
}
+static char *
+parseProcessorInfo(char *base, virSysinfoDefPtr ret)
As with previous patch, please add 'virSysinfo' prefix on all
method names
+static void
+ProcessorInfoFormat(virSysinfoDefPtr def, const char *prefix, virBufferPtr buf)
diff --git a/src/util/sysinfo.h b/src/util/sysinfo.h
index f69b76c..f098e9d 100644
--- a/src/util/sysinfo.h
+++ b/src/util/sysinfo.h
@@ -33,6 +33,22 @@ enum virSysinfoType {
VIR_SYSINFO_LAST
};
+typedef struct _virProcessorinfoDef virProcessorinfoDef;
+typedef virProcessorinfoDef *virProcessorinfoDefPtr;
+struct _virProcessorinfoDef {
+ char *processor_socket_destination;
+ char *processor_type;
+ char *processor_family;
+ char *processor_manufacturer;
+ char *processor_signature;
+ char *processor_version;
+ char *processor_external_clock;
+ char *processor_max_speed;
+ char *processor_status;
+ char *processor_serial_number;
+ char *processor_part_number;
+};
All these strings are leaked AFAICT
Also can we name this struct
virSysinfoProcessorDef
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
9:28 p.m.
New subject: [libvirt] [PATCH 2/3] Add Processor Information to virSysinfoRead() from dmidecode type 4
Hi,
On Fri, 24 Jun 2011 15:24:00 +0100
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Tue, Jun 21, 2011 at 01:20:30PM +0900, Minoru Usui wrote:
> Add Processor Information to virSysinfoRead() from dmidecode type 4
>
> Signed-off-by: Minoru Usui <usui(a)mxm.nes.nec.co.jp>
> ---
> src/util/sysinfo.c | 207 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> src/util/sysinfo.h | 19 +++++
> 2 files changed, 225 insertions(+), 1 deletions(-)
>
> diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
> index 53122f7..a1eb92b 100644
> --- a/src/util/sysinfo.c
> +++ b/src/util/sysinfo.c
> @@ -72,6 +72,9 @@ void virSysinfoDefFree(virSysinfoDefPtr def)
> VIR_FREE(def->system_uuid);
> VIR_FREE(def->system_sku);
> VIR_FREE(def->system_family);
> +
> + VIR_FREE(def->processor);
This frees the 'virProcessinfoDef' instance, but what
about all the allocated strings inside it ?
I'm sorry. It's my fault.
But Mr. Veillard fixed these memory leaks. Thank a lot.
> +
> VIR_FREE(def);
> }
>
> @@ -190,6 +193,107 @@ no_memory:
> return NULL;
> }
>
> +static char *
> +parseProcessorInfo(char *base, virSysinfoDefPtr ret)
As with previous patch, please add 'virSysinfo' prefix on all
method names
OK.
I'll add prefix.
> +static void
> +ProcessorInfoFormat(virSysinfoDefPtr def, const char *prefix, virBufferPtr buf)
> diff --git a/src/util/sysinfo.h b/src/util/sysinfo.h
> index f69b76c..f098e9d 100644
> --- a/src/util/sysinfo.h
> +++ b/src/util/sysinfo.h
> @@ -33,6 +33,22 @@ enum virSysinfoType {
> VIR_SYSINFO_LAST
> };
>
> +typedef struct _virProcessorinfoDef virProcessorinfoDef;
> +typedef virProcessorinfoDef *virProcessorinfoDefPtr;
> +struct _virProcessorinfoDef {
> + char *processor_socket_destination;
> + char *processor_type;
> + char *processor_family;
> + char *processor_manufacturer;
> + char *processor_signature;
> + char *processor_version;
> + char *processor_external_clock;
> + char *processor_max_speed;
> + char *processor_status;
> + char *processor_serial_number;
> + char *processor_part_number;
> +};
All these strings are leaked AFAICT
Also can we name this struct
virSysinfoProcessorDef
OK.
I'll change it.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
Minoru Usui <usui(a)mxm.nes.nec.co.jp>
11:21 p.m.
New subject: [libvirt] [PATCH 3/3] Add Memory Device Information to virSysinfoRead() from dmidecode type 17
Add Memory Device Information to virSysinfoRead() from dmidecode type 17
Signed-off-by: Minoru Usui <usui(a)mxm.nes.nec.co.jp>
---
src/util/sysinfo.c | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++-
src/util/sysinfo.h | 18 +++++
2 files changed, 212 insertions(+), 1 deletions(-)
diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
index a1eb92b..7ebf355 100644
--- a/src/util/sysinfo.c
+++ b/src/util/sysinfo.c
@@ -74,6 +74,7 @@ void virSysinfoDefFree(virSysinfoDefPtr def)
VIR_FREE(def->system_family);
VIR_FREE(def->processor);
+ VIR_FREE(def->memory);
VIR_FREE(def);
}
@@ -294,6 +295,104 @@ no_memory:
return NULL;
}
+static char *
+parseMemoryDeviceInfo(char *base, virSysinfoDefPtr ret)
+{
+ char *cur, *eol, *tmp_base;
+ virMemoryDeviceinfoDefPtr memory;
+
+ while ((tmp_base = strstr(base, "Memory Device")) != NULL) {
+ base = tmp_base;
+
+ if (VIR_EXPAND_N(ret->memory, ret->nmemory, 1) < 0) {
+ goto no_memory;
+ }
+ memory = &ret->memory[ret->nmemory - 1];
+
+ if ((cur = strstr(base, "Size: ")) != NULL) {
+ cur += 6;
+ eol = strchr(cur, '\n');
+ if (STREQLEN(cur, "No Module Installed", eol - cur))
+ goto next;
+
+ if ((eol) &&
+ ((memory->memory_size = strndup(cur, eol - cur)) == NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Form Factor: ")) != NULL) {
+ cur += 13;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((memory->memory_form_factor = strndup(cur, eol - cur)) == NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Locator: ")) != NULL) {
+ cur += 9;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((memory->memory_locator = strndup(cur, eol - cur)) == NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Bank Locator: ")) != NULL) {
+ cur += 14;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((memory->memory_bank_locator = strndup(cur, eol - cur)) == NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Type: ")) != NULL) {
+ cur += 6;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((memory->memory_type = strndup(cur, eol - cur)) == NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Type Detail: ")) != NULL) {
+ cur += 13;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((memory->memory_type_detail = strndup(cur, eol - cur)) == NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Speed: ")) != NULL) {
+ cur += 7;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((memory->memory_speed = strndup(cur, eol - cur)) == NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Manufacturer: ")) != NULL) {
+ cur += 14;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((memory->memory_manufacturer = strndup(cur, eol - cur)) == NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Serial Number: ")) != NULL) {
+ cur += 15;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((memory->memory_serial_number = strndup(cur, eol - cur)) == NULL))
+ goto no_memory;
+ }
+ if ((cur = strstr(base, "Part Number: ")) != NULL) {
+ cur += 13;
+ eol = strchr(cur, '\n');
+ if ((eol) &&
+ ((memory->memory_part_number = strndup(cur, eol - cur)) == NULL))
+ goto no_memory;
+ }
+
+ next:
+ base = eol + 1;
+ }
+
+ return base;
+
+no_memory:
+ return NULL;
+}
+
virSysinfoDefPtr
virSysinfoRead(void) {
char *path, *base;
@@ -309,7 +408,7 @@ virSysinfoRead(void) {
return NULL;
}
- cmd = virCommandNewArgList(path, "-q", "-t", "0,1,4",
NULL);
+ cmd = virCommandNewArgList(path, "-q", "-t",
"0,1,4,17", NULL);
VIR_FREE(path);
virCommandSetOutputBuffer(cmd, &outbuf);
if (virCommandRun(cmd, NULL) < 0) {
@@ -337,6 +436,11 @@ virSysinfoRead(void) {
if ((base = parseProcessorInfo(base, ret)) == NULL)
goto no_memory;
+ ret->nmemory = 0;
+ ret->memory = NULL;
+ if ((base = parseMemoryDeviceInfo(base, ret)) == NULL)
+ goto no_memory;
+
cleanup:
VIR_FREE(outbuf);
virCommandFree(cmd);
@@ -543,6 +647,94 @@ ProcessorInfoFormat(virSysinfoDefPtr def, const char *prefix,
virBufferPtr buf)
return;
}
+static void
+MemoryDeviceInfoFormat(virSysinfoDefPtr def, const char *prefix, virBufferPtr buf)
+{
+ int i;
+ int len = strlen(prefix);
+ virMemoryDeviceinfoDefPtr memory;
+
+ for (i = 0; i < def->nmemory; i++) {
+ memory = &def->memory[i];
+
+ if ((memory->memory_size != NULL) ||
+ (memory->memory_form_factor != NULL) ||
+ (memory->memory_locator != NULL) ||
+ (memory->memory_bank_locator != NULL) ||
+ (memory->memory_type != NULL) ||
+ (memory->memory_type_detail != NULL) ||
+ (memory->memory_speed != NULL) ||
+ (memory->memory_manufacturer != NULL) ||
+ (memory->memory_serial_number != NULL) ||
+ (memory->memory_part_number != NULL)) {
+ virBufferAsprintf(buf, "%s <memory_device>\n", prefix);
+ if (memory->memory_size != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='size'>%s</entry>\n",
+ memory->memory_size);
+ }
+ if (memory->memory_form_factor != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='form_factor'>%s</entry>\n",
+ memory->memory_form_factor);
+ }
+ if (memory->memory_locator != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='locator'>%s</entry>\n",
+ memory->memory_locator);
+ }
+ if (memory->memory_bank_locator != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='bank_locator'>%s</entry>\n",
+ memory->memory_bank_locator);
+ }
+ if (memory->memory_type != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='type'>%s</entry>\n",
+ memory->memory_type);
+ }
+ if (memory->memory_type_detail != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='type_detail'>%s</entry>\n",
+ memory->memory_type_detail);
+ }
+ if (memory->memory_speed != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='speed'>%s</entry>\n",
+ memory->memory_speed);
+ }
+ if (memory->memory_manufacturer != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='manufacturer'>%s</entry>\n",
+ memory->memory_manufacturer);
+ }
+ if (memory->memory_serial_number != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='serial_number'>%s</entry>\n",
+ memory->memory_serial_number);
+ }
+ if (memory->memory_part_number != NULL) {
+ virBufferAdd(buf, prefix, len);
+ virBufferEscapeString(buf,
+ " <entry
name='part_number'>%s</entry>\n",
+ memory->memory_part_number);
+ }
+ virBufferAsprintf(buf, "%s </memory_device>\n", prefix);
+ }
+ }
+
+ return;
+}
+
/**
* virSysinfoFormat:
* @def: structure to convert to xml string
@@ -569,6 +761,7 @@ virSysinfoFormat(virSysinfoDefPtr def, const char *prefix)
BIOSInfoFormat(def, prefix, &buf);
SystemInfoFormat(def, prefix, &buf);
ProcessorInfoFormat(def, prefix, &buf);
+ MemoryDeviceInfoFormat(def, prefix, &buf);
virBufferAsprintf(&buf, "%s</sysinfo>\n", prefix);
diff --git a/src/util/sysinfo.h b/src/util/sysinfo.h
index f098e9d..a15c5ac 100644
--- a/src/util/sysinfo.h
+++ b/src/util/sysinfo.h
@@ -49,6 +49,21 @@ struct _virProcessorinfoDef {
char *processor_part_number;
};
+typedef struct _virMemoryDeviceinfoDef virMemoryDeviceinfoDef;
+typedef virMemoryDeviceinfoDef *virMemoryDeviceinfoDefPtr;
+struct _virMemoryDeviceinfoDef {
+ char *memory_size;
+ char *memory_form_factor;
+ char *memory_locator;
+ char *memory_bank_locator;
+ char *memory_type;
+ char *memory_type_detail;
+ char *memory_speed;
+ char *memory_manufacturer;
+ char *memory_serial_number;
+ char *memory_part_number;
+};
+
typedef struct _virSysinfoDef virSysinfoDef;
typedef virSysinfoDef *virSysinfoDefPtr;
struct _virSysinfoDef {
@@ -69,6 +84,9 @@ struct _virSysinfoDef {
size_t nprocessor;
virProcessorinfoDefPtr processor;
+
+ size_t nmemory;
+ virMemoryDeviceinfoDefPtr memory;
};
virSysinfoDefPtr virSysinfoRead(void);
--
1.7.1
9:25 a.m.
New subject: [libvirt] [PATCH 3/3] Add Memory Device Information to virSysinfoRead() from dmidecode type 17
On Tue, Jun 21, 2011 at 01:21:34PM +0900, Minoru Usui wrote:
Add Memory Device Information to virSysinfoRead() from dmidecode type
17
Signed-off-by: Minoru Usui <usui(a)mxm.nes.nec.co.jp>
---
src/util/sysinfo.c | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++-
src/util/sysinfo.h | 18 +++++
2 files changed, 212 insertions(+), 1 deletions(-)
diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
index a1eb92b..7ebf355 100644
--- a/src/util/sysinfo.c
+++ b/src/util/sysinfo.c
@@ -74,6 +74,7 @@ void virSysinfoDefFree(virSysinfoDefPtr def)
VIR_FREE(def->system_family);
VIR_FREE(def->processor);
+ VIR_FREE(def->memory);
Again, I believe this leaks all the strings inside the
struct
+static char *
+parseMemoryDeviceInfo(char *base, virSysinfoDefPtr ret)
Same note about method naming with 'virSysinfo' prefix
diff --git a/src/util/sysinfo.h b/src/util/sysinfo.h
index f098e9d..a15c5ac 100644
--- a/src/util/sysinfo.h
+++ b/src/util/sysinfo.h
@@ -49,6 +49,21 @@ struct _virProcessorinfoDef {
char *processor_part_number;
};
+typedef struct _virMemoryDeviceinfoDef virMemoryDeviceinfoDef;
+typedef virMemoryDeviceinfoDef *virMemoryDeviceinfoDefPtr;
+struct _virMemoryDeviceinfoDef {
+ char *memory_size;
+ char *memory_form_factor;
+ char *memory_locator;
+ char *memory_bank_locator;
+ char *memory_type;
+ char *memory_type_detail;
+ char *memory_speed;
+ char *memory_manufacturer;
+ char *memory_serial_number;
+ char *memory_part_number;
+};
Can we rename this struct to 'virSysinfoMemoryDef'
Do we really want all these fields to be strings, or can some of them
be stored as 'unsigned long long' perhaps for greater validation ?
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
9:41 p.m.
New subject: [libvirt] [PATCH 3/3] Add Memory Device Information to virSysinfoRead() from dmidecode type 17
On Fri, 24 Jun 2011 15:25:42 +0100
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Tue, Jun 21, 2011 at 01:21:34PM +0900, Minoru Usui wrote:
> Add Memory Device Information to virSysinfoRead() from dmidecode type 17
>
> Signed-off-by: Minoru Usui <usui(a)mxm.nes.nec.co.jp>
> ---
> src/util/sysinfo.c | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> src/util/sysinfo.h | 18 +++++
> 2 files changed, 212 insertions(+), 1 deletions(-)
>
> diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
> index a1eb92b..7ebf355 100644
> --- a/src/util/sysinfo.c
> +++ b/src/util/sysinfo.c
> @@ -74,6 +74,7 @@ void virSysinfoDefFree(virSysinfoDefPtr def)
> VIR_FREE(def->system_family);
>
> VIR_FREE(def->processor);
> + VIR_FREE(def->memory);
Again, I believe this leaks all the strings inside the
struct
> +static char *
> +parseMemoryDeviceInfo(char *base, virSysinfoDefPtr ret)
Same note about method naming with 'virSysinfo' prefix
I'll fix it.
> diff --git a/src/util/sysinfo.h b/src/util/sysinfo.h
> index f098e9d..a15c5ac 100644
> --- a/src/util/sysinfo.h
> +++ b/src/util/sysinfo.h
> @@ -49,6 +49,21 @@ struct _virProcessorinfoDef {
> char *processor_part_number;
> };
>
> +typedef struct _virMemoryDeviceinfoDef virMemoryDeviceinfoDef;
> +typedef virMemoryDeviceinfoDef *virMemoryDeviceinfoDefPtr;
> +struct _virMemoryDeviceinfoDef {
> + char *memory_size;
> + char *memory_form_factor;
> + char *memory_locator;
> + char *memory_bank_locator;
> + char *memory_type;
> + char *memory_type_detail;
> + char *memory_speed;
> + char *memory_manufacturer;
> + char *memory_serial_number;
> + char *memory_part_number;
> +};
Can we rename this struct to 'virSysinfoMemoryDef'
Do we really want all these fields to be strings, or can some of them
be stored as 'unsigned long long' perhaps for greater validation ?
virCommandRun() returns strings, and we prints out these information to strings. (XML)
So, I think it is reasonable, isn't it?
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
Minoru Usui <usui(a)mxm.nes.nec.co.jp>
8:36 p.m.
On Tue, Jun 21, 2011 at 01:17:38PM +0900, Minoru Usui wrote:
Hi,
I propose to add hardware Processor/MemoryDevice information to virSysinfoRead() from
SMBIOS.
I want to get host machine's these infomations through virConnectGetSysinfo().
I think it's useful to managing many host machines,
because administrator can centrally manage the host machines which consists of
Processor/MemoryDevice parts.
This patch set adds DMI type4, 17 information(dmidecode -t4,17) to virSysinfoRead().
I hope this patch set adds v0.9.3, if possible.
it's a bit late. What is the expected output ? You should have added
that for easier review,
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/
11:16 p.m.
Hi, Daniel
Thank you for you reply.
On Thu, 23 Jun 2011 09:36:11 +0800
Daniel Veillard <veillard(a)redhat.com> wrote:
On Tue, Jun 21, 2011 at 01:17:38PM +0900, Minoru Usui wrote:
> Hi,
>
> I propose to add hardware Processor/MemoryDevice information to virSysinfoRead()
from SMBIOS.
> I want to get host machine's these infomations through virConnectGetSysinfo().
>
> I think it's useful to managing many host machines,
> because administrator can centrally manage the host machines which consists of
> Processor/MemoryDevice parts.
>
> This patch set adds DMI type4, 17 information(dmidecode -t4,17) to
virSysinfoRead().
> I hope this patch set adds v0.9.3, if possible.
it's a bit late. What is the expected output ? You should have added
that for easier review,
I'm sorry for late posting and lack explanation.
Expected output is below.
I add "<processor>" and "<memory_device>" part.
"<processor>" part gets "Processor Information" part of
dmidecode -t4,
and "<memory_device>" part gets existing "Memory Device" part of
dmidecode -t17.
Existing "Memory Device" part means "Size" member is not "No
Module Installed".
How about this?
-------------------------------------------------------------
# ./build/tools/virsh sysinfo
<sysinfo type='smbios'>
<bios>
<entry name='vendor'>Phoenix Technologies LTD</entry>
<entry name='version'>1.0.5S46</entry>
<entry name='date'>12/21/2006</entry>
</bios>
<system>
<entry name='manufacturer'>NEC</entry>
<entry name='product'>Express5800/120Rg-1 [N8100-1241]</entry>
<entry name='version'>FR1.5</entry>
<entry name='serial'>6800109</entry>
<entry name='uuid'>BC466200-2232-11DB-8001-001617617E41</entry>
<entry name='sku'>Not Specified</entry>
<entry name='family'>Not Specified</entry>
</system>
<processor>
<entry name='socket_destination'>CPU#1 </entry>
<entry name='type'>Central Processor</entry>
<entry name='family'>Xeon</entry>
<entry name='manufacturer'>Intel Corporation</entry>
<entry name='signature'>Type 0, Family 6, Model 15, Stepping
7</entry>
<entry name='version'>Intel(R) Xeon(R) CPU E5345 @ 2.33GHz
</entry>
<entry name='external_clock'>333 MHz</entry>
<entry name='max_speed'>3000 MHz</entry>
<entry name='status'>Populated, Enabled</entry>
<entry name='serial_number'> </entry>
<entry name='part_number'> </entry>
</processor>
<processor>
<entry name='socket_destination'>CPU#2 </entry>
<entry name='type'>Central Processor</entry>
<entry name='family'>Xeon</entry>
<entry name='manufacturer'>Intel Corporation</entry>
<entry name='signature'>Type 0, Family 6, Model 15, Stepping
7</entry>
<entry name='version'>Intel(R) Xeon(R) CPU E5345 @ 2.33GHz
</entry>
<entry name='external_clock'>333 MHz</entry>
<entry name='max_speed'>3000 MHz</entry>
<entry name='status'>Populated, Enabled</entry>
<entry name='serial_number'> </entry>
<entry name='part_number'> </entry>
</processor>
: continue number of "Processor Information" of dmidecode times
<memory_device>
<entry name='size'>1024 MB</entry>
<entry name='form_factor'>DIMM</entry>
<entry name='locator'>DIMM 11</entry>
<entry name='bank_locator'>BANK 1</entry>
<entry name='type'>DDR2</entry>
<entry name='type_detail'>Synchronous</entry>
<entry name='speed'>533 MHz</entry>
<entry name='manufacturer'>80CE </entry>
<entry name='serial_number'>030C5822</entry>
<entry name='part_number'>M395T2953CZ4-CE61 </entry>
</memory_device>
<memory_device>
<entry name='size'>1024 MB</entry>
<entry name='form_factor'>DIMM</entry>
<entry name='locator'>DIMM 21</entry>
<entry name='bank_locator'>BANK 1</entry>
<entry name='type'>DDR2</entry>
<entry name='type_detail'>Synchronous</entry>
<entry name='speed'>533 MHz</entry>
<entry name='manufacturer'>80CE </entry>
<entry name='serial_number'>030C5638</entry>
<entry name='part_number'>M395T2953CZ4-CE61 </entry>
</memory_device>
: continue number of existing "Memory Device" times
</sysinfo>
-------------------------------------------------------------
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/
--
Minoru Usui <usui(a)mxm.nes.nec.co.jp>
2:20 a.m.
On Thu, Jun 23, 2011 at 01:16:36PM +0900, Minoru Usui wrote:
Hi, Daniel
Thank you for you reply.
[...]
I'm sorry for late posting and lack explanation.
Expected output is below.
I add "<processor>" and "<memory_device>" part.
"<processor>" part gets "Processor Information" part of
dmidecode -t4,
and "<memory_device>" part gets existing "Memory Device" part
of dmidecode -t17.
Existing "Memory Device" part means "Size" member is not "No
Module Installed".
How about this?
-------------------------------------------------------------
# ./build/tools/virsh sysinfo
<sysinfo type='smbios'>
<bios>
<entry name='vendor'>Phoenix Technologies LTD</entry>
<entry name='version'>1.0.5S46</entry>
<entry name='date'>12/21/2006</entry>
</bios>
<system>
<entry name='manufacturer'>NEC</entry>
<entry name='product'>Express5800/120Rg-1 [N8100-1241]</entry>
<entry name='version'>FR1.5</entry>
<entry name='serial'>6800109</entry>
<entry name='uuid'>BC466200-2232-11DB-8001-001617617E41</entry>
<entry name='sku'>Not Specified</entry>
<entry name='family'>Not Specified</entry>
</system>
[...]
<processor>
<entry name='socket_destination'>CPU#2 </entry>
<entry name='type'>Central Processor</entry>
<entry name='family'>Xeon</entry>
<entry name='manufacturer'>Intel Corporation</entry>
<entry name='signature'>Type 0, Family 6, Model 15, Stepping
7</entry>
<entry name='version'>Intel(R) Xeon(R) CPU E5345 @ 2.33GHz
</entry>
<entry name='external_clock'>333 MHz</entry>
<entry name='max_speed'>3000 MHz</entry>
<entry name='status'>Populated, Enabled</entry>
<entry name='serial_number'> </entry>
<entry name='part_number'> </entry>
</processor>
[...]
<memory_device>
<entry name='size'>1024 MB</entry>
<entry name='form_factor'>DIMM</entry>
<entry name='locator'>DIMM 21</entry>
<entry name='bank_locator'>BANK 1</entry>
<entry name='type'>DDR2</entry>
<entry name='type_detail'>Synchronous</entry>
<entry name='speed'>533 MHz</entry>
<entry name='manufacturer'>80CE </entry>
<entry name='serial_number'>030C5638</entry>
<entry name='part_number'>M395T2953CZ4-CE61 </entry>
</memory_device>
Okay, I reviewed the patches, they are okay, except the freeing was
leaking all the strings from the memory and processor arrays. it was
easy to fix.
I think it's okay to add this based on the fact we want extend libvirt
toward being an entry point for management tools, but that's on the
edge since it's not really specific to virtualization (and where needed
i.e. CPU flags and memory hierarchy) those informations are already
provided.
My question is "do you need all those informations ???", my concern
is that already on my small desktop the output is 3KB and I assume on a
very large box we could hit hundreds of kilobytes.
Also empty entries (i.e. zero lenght or spaces) should IMHO be
removed.
I'm ready to commit those with the memory leak fixes, but a bit more
feedback would be welcome :-)
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/
9:38 a.m.
On Fri, Jun 24, 2011 at 03:20:29PM +0800, Daniel Veillard wrote:
On Thu, Jun 23, 2011 at 01:16:36PM +0900, Minoru Usui wrote:
[...]
My question is "do you need all those informations ???",
my concern
is that already on my small desktop the output is 3KB and I assume on a
very large box we could hit hundreds of kilobytes.
Also empty entries (i.e. zero lenght or spaces) should IMHO be
removed.
I'm ready to commit those with the memory leak fixes, but a bit more
feedback would be welcome :-)
I just commited and just seeing Dan comments now, I think we will
clean this up beginning of next week
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/
8:32 p.m.
Hi,
On Fri, 24 Jun 2011 22:38:44 +0800
Daniel Veillard <veillard(a)redhat.com> wrote:
On Fri, Jun 24, 2011 at 03:20:29PM +0800, Daniel Veillard wrote:
> On Thu, Jun 23, 2011 at 01:16:36PM +0900, Minoru Usui wrote:
[...]
> My question is "do you need all those informations ???", my concern
> is that already on my small desktop the output is 3KB and I assume on a
> very large box we could hit hundreds of kilobytes.
> Also empty entries (i.e. zero lenght or spaces) should IMHO be
> removed.
>
> I'm ready to commit those with the memory leak fixes, but a bit more
> feedback would be welcome :-)
I just commited and just seeing Dan comments now, I think we will
clean this up beginning of next week
I'm sorry for late reply.
Thank you for commiting my patch.
I'll refrect Dan's comment.
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/
--
Minoru Usui <usui(a)mxm.nes.nec.co.jp>
4934
days inactive
4940
days old
14 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Minoru Usui