[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] udev: Don't try to dump DMI on non-intel archs



On 20.07.2011 16:20, Eric Blake wrote:
On 07/20/2011 08:09 AM, Michal Privoznik wrote:
That depends on how things behave on other arches. I don't have ready
access to such an architecture, which is why I'm asking - do we already
gracefully handle any failures on code paths that try to use dmidecode,
or are there currently code paths that make unconditional use of
dmidecode (and fail) which should be made arch-conditional?

It's pointless to write a patch until we know what behavior we're
patching.

So I've managed to run libvirt on ppc64. Here are the results:


# virsh sysinfo
error: failed to get sysinfo
error: unsupported configuration: Host SMBIOS information is not
available

Reasonable - we're gracefully handling the lack of information, and with
an appropriate error message.

and in logs:
05:25:40.682: 29653: error : virSysinfoRead:462 : internal error Failed
to find path for dmidecode binary

Not so nice. "internal error" in a log message always sparks worry among
people, even if in this case it did not stop libvirt from doing useful
work. I would be in favor of a patch to src/util/sysinfo.c that changes
the #ifdef WIN32 around virSysinfoRead() stub that fails with ENOSYS
instead of VIR_ERR_INTERNAL_ERROR to also non-x86 arches. Could you test
this, to see whether that makes the log less scary?

diff --git i/src/util/sysinfo.c w/src/util/sysinfo.c
index 2c8e687..9cd6849 100644
--- i/src/util/sysinfo.c
+++ w/src/util/sysinfo.c
@@ -113,7 +113,10 @@ void virSysinfoDefFree(virSysinfoDefPtr def)
*
* Returns: a filled up sysinfo structure or NULL in case of error
*/
-#ifdef WIN32
+#if defined(WIN32) || \
+ defined(__x86_64__) || \
+ defined(__i386__) || \
+ defined(__amd64__)
virSysinfoDefPtr
virSysinfoRead(void) {
/*
@@ -125,7 +128,7 @@ virSysinfoRead(void) {
return NULL;
}

-#else /* !WIN32 */
+#else /* !WIN32 && x86 */

static char *
virSysinfoParseBIOS(char *base, virSysinfoDefPtr ret)


In fact, we need a patch with complement condition:

diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
index 2c8e687..6625cae 100644
--- a/src/util/sysinfo.c
+++ b/src/util/sysinfo.c
@@ -113,7 +113,10 @@ void virSysinfoDefFree(virSysinfoDefPtr def)
  *
  * Returns: a filled up sysinfo structure or NULL in case of error
  */
-#ifdef WIN32
+#if defined(WIN32) || \
+    !(defined(__x86_64__) || \
+      defined(__i386__) ||   \
+      defined(__amd64__))
 virSysinfoDefPtr
 virSysinfoRead(void) {
     /*
@@ -125,7 +128,7 @@ virSysinfoRead(void) {
     return NULL;
 }

-#else /* !WIN32 */
+#else /* !WIN32 && x86 */

 static char *
 virSysinfoParseBIOS(char *base, virSysinfoDefPtr ret)
@@ -509,7 +512,7 @@ no_memory:
     ret = NULL;
     goto cleanup;
 }
-#endif /* !WIN32 */
+#endif /* !WIN32 && x86 */

 static void
 virSysinfoBIOSFormat(virSysinfoDefPtr def, const char *prefix,


With this patch we transfer to error:
10:51:36.515: 4412: error : virSysinfoRead:127 : Host sysinfo extraction not supported on this platform: Function not implemented

I think this is the right way of dealing with this, isn't it?

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]