[libvirt] [PATCH][PowerPC] Implement sysinfo for libvirt on PowerPC

Eric Blake eblake at redhat.com
Thu Feb 9 23:03:26 UTC 2012


On 02/09/2012 01:47 AM, Prerna Saxena wrote:
> From: Prerna Saxena <prerna at linux.vnet.ibm.com>
> Date: Tue, 7 Feb 2012 16:55:26 +0530
> Subject: [PATCH] Implement sysinfo on PowerPC.
> 
> Libvirt on x86 parses 'dmidecode' to gather characteristics of host
> system, which are then reflected to libvirt users by virSysinfoRead(),
> invoked by 'virsh sysinfo'.
> This patch implements it on PowerPC by reading /proc/cpuinfo.
> 
> The presently available fields in 'sysinfo' are strongly tied to
> dmidecode output fields. This patch attempts to retrofit the
> information available in PowerPC to appropriate sysinfo fields. I will
> be happy to change the organization of this information to if there
> are expected outputs for individual fields. (I couldnt find any
> documentation which explained what each sysinfo field was expected
> to convey.)

At this point, I think it might be worth waiting until post-0.9.10, and
rebasing this on the latest.  In particular,

> +
> +#if defined(__powerpc__)
> +static char*
> +virSysinfoParseSystem(char *base, virSysinfoDefPtr ret)

We just recently changed the signature of this function in the x86 case,
so the two implementations should probably have a similar signature.

> +
> +    ret->system_manufacturer = NULL;
> +    ret->system_product = NULL;

Setting these to NULL is redundant, since ret was just freshly allocated.

> +    ret->system_uuid = NULL;

Is there any alternative way to obtain the host UUID that we could fill
in here?

> +/* virSysinfoRead for PowerPC */
> +virSysinfoDefPtr
> +virSysinfoRead(void) {
> +    virSysinfoDefPtr ret = NULL;
> +    char *outbuf = NULL;
> +
> +    if (VIR_ALLOC(ret) < 0)
> +        goto no_memory;
> +
> +    /* mark irrelevant fields as 'NULL' */
> +    ret->bios_vendor = NULL;
> +    ret->bios_version = NULL;
> +    ret->bios_date = NULL;
> +    ret->bios_release = NULL;

Again, redundant, since VIR_ALLOC() already took care of that.

> +
> +    if(virFileReadAll(CPUINFO, 2048, &outbuf) < 0) {
> +        virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
> +                             _("Failed to open %s"),CPUINFO);

Nit: space after ','.

Overall, the idea looks nice.

I'm also wondering how much of that implementation of parsing
/proc/cpuinfo can be reused on x86 in the cases where dmidecode is not
accessible (such as when running qemu:///session as non-root)?  Filling
out what we can seems like a good idea.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120209/fabda5b6/attachment-0001.sig>


More information about the libvir-list mailing list