9:52 a.m.
DMI is Intel & Intel-compatible specific. Don't try to dump information
on non-compatible architectures, which results only in error message in
logs.
---
NB: libsmbios is exclusively for x86_64 ia64 %{ix86}
src/node_device/node_device_udev.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index a6b5b2e..2c5d016 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1477,6 +1477,8 @@ out:
}
+/* DMI is intel-compatible specific */
+#if defined(__x86_64__) || defined(__i386__) || defined(__amd64__)
static void
udevGetDMIData(union _virNodeDevCapData *data)
{
@@ -1549,6 +1551,7 @@ out:
}
return;
}
+#endif
static int udevSetupSystemDev(void)
@@ -1573,7 +1576,9 @@ static int udevSetupSystemDev(void)
goto out;
}
+#if defined(__x86_64__) || defined(__i386__) || defined(__amd64__)
udevGetDMIData(&def->caps->data);
+#endif
dev = virNodeDeviceAssignDef(&driverState->devs, def);
if (dev == NULL) {
--
1.7.5.rc3
10:01 a.m.
On 07/19/2011 08:52 AM, Michal Privoznik wrote:
DMI is Intel& Intel-compatible specific. Don't try to dump
information
on non-compatible architectures, which results only in error message in
logs.
---
NB: libsmbios is exclusively for x86_64 ia64 %{ix86}
src/node_device/node_device_udev.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
ACK.
But do we also need to patch src/util/sysinfo.c, which makes callouts to
dmidecode(1)? That is, if libsmbios is arch-specific, then I imagine
that so is dmidecode, and do we gracefully handle dmidecode failures in
that case?
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
10:12 a.m.
On 19.07.2011 17:01, Eric Blake wrote:
On 07/19/2011 08:52 AM, Michal Privoznik wrote:
> DMI is Intel& Intel-compatible specific. Don't try to dump information
> on non-compatible architectures, which results only in error message in
> logs.
> ---
> NB: libsmbios is exclusively for x86_64 ia64 %{ix86}
>
> src/node_device/node_device_udev.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
ACK.
But do we also need to patch src/util/sysinfo.c, which makes callouts to
dmidecode(1)? That is, if libsmbios is arch-specific, then I imagine
that so is dmidecode, and do we gracefully handle dmidecode failures in
that case?
You're right. dmidecode is arch-specific as well. Right now we just fail
in running it. Is it enough or should that be enclosed in conditional
compilation as well?
Michal
10:17 a.m.
On 07/19/2011 09:12 AM, Michal Privoznik wrote:
On 19.07.2011 17:01, Eric Blake wrote:
> On 07/19/2011 08:52 AM, Michal Privoznik wrote:
>> DMI is Intel& Intel-compatible specific. Don't try to dump information
>> on non-compatible architectures, which results only in error message in
>> logs.
>> ---
>> NB: libsmbios is exclusively for x86_64 ia64 %{ix86}
>>
>> src/node_device/node_device_udev.c | 5 +++++
>> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> ACK.
>
> But do we also need to patch src/util/sysinfo.c, which makes callouts to
> dmidecode(1)? That is, if libsmbios is arch-specific, then I imagine
> that so is dmidecode, and do we gracefully handle dmidecode failures in
> that case?
>
You're right. dmidecode is arch-specific as well. Right now we just fail
in running it. Is it enough or should that be enclosed in conditional
compilation as well?
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.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
9:09 a.m.
On 19.07.2011 17:17, Eric Blake wrote:
On 07/19/2011 09:12 AM, Michal Privoznik wrote:
> On 19.07.2011 17:01, Eric Blake wrote:
>> On 07/19/2011 08:52 AM, Michal Privoznik wrote:
>>> DMI is Intel& Intel-compatible specific. Don't try to dump
information
>>> on non-compatible architectures, which results only in error message in
>>> logs.
>>> ---
>>> NB: libsmbios is exclusively for x86_64 ia64 %{ix86}
>>>
>>> src/node_device/node_device_udev.c | 5 +++++
>>> 1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> ACK.
>>
>> But do we also need to patch src/util/sysinfo.c, which makes callouts to
>> dmidecode(1)? That is, if libsmbios is arch-specific, then I imagine
>> that so is dmidecode, and do we gracefully handle dmidecode failures in
>> that case?
>>
>
> You're right. dmidecode is arch-specific as well. Right now we just fail
> in running it. Is it enough or should that be enclosed in conditional
> compilation as well?
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
and in logs:
05:25:40.682: 29653: error : virSysinfoRead:462 : internal error Failed
to find path for dmidecode binary
which get logged on daemon startup, not that virsh command.
Michal
9:20 a.m.
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)
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
9:54 a.m.
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
9:58 a.m.
On 07/20/2011 08:54 AM, Michal Privoznik wrote:
> -#ifdef WIN32
> +#if defined(WIN32) || \
> + defined(__x86_64__) || \
> + defined(__i386__) || \
> + defined(__amd64__)
>
In fact, we need a patch with complement condition:
Doh, serves me right for typing without testing.
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__))
Much nicer.
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?
Yes, that looks nicer. ACK to committing this as a patch.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
10:16 a.m.
On 20.07.2011 16:58, Eric Blake wrote:
On 07/20/2011 08:54 AM, Michal Privoznik wrote:
>> -#ifdef WIN32
>> +#if defined(WIN32) || \
>> + defined(__x86_64__) || \
>> + defined(__i386__) || \
>> + defined(__amd64__)
>>
> In fact, we need a patch with complement condition:
Doh, serves me right for typing without testing.
>
> 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__))
Much nicer.
>
> 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?
Yes, that looks nicer. ACK to committing this as a patch.
Pushed.
Michal
9:25 a.m.
On 07/19/2011 09:01 AM, Eric Blake wrote:
But do we also need to patch src/util/sysinfo.c, which makes callouts
to
dmidecode(1)? That is, if libsmbios is arch-specific, then I imagine
that so is dmidecode, and do we gracefully handle dmidecode failures in
that case?
Oh, and while I'm thinking about it:
on x86 architectures with newer kernels, the kernel is starting to
provide various SMBIOS information to user space via sysfs, without
requiring the running of root-only dmidecode. We need to teach libvirt
to try that method, so that qemu:///session can take advantage of
copying host SMBIOS information to the guest.
https://www.redhat.com/archives/virt-tools-list/2011-July/msg00012.html
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
10:41 a.m.
On 20.07.2011 16:25, Eric Blake wrote:
On 07/19/2011 09:01 AM, Eric Blake wrote:
> But do we also need to patch src/util/sysinfo.c, which makes callouts to
> dmidecode(1)? That is, if libsmbios is arch-specific, then I imagine
> that so is dmidecode, and do we gracefully handle dmidecode failures in
> that case?
Oh, and while I'm thinking about it:
on x86 architectures with newer kernels, the kernel is starting to
provide various SMBIOS information to user space via sysfs, without
requiring the running of root-only dmidecode. We need to teach libvirt
to try that method, so that qemu:///session can take advantage of
copying host SMBIOS information to the guest.
https://www.redhat.com/archives/virt-tools-list/2011-July/msg00012.html
well, sysfs currently offer much less info than dmidecode. But it still
might be worth of expanding it.
Michal
4874
days inactive
4875
days old
10 comments
2 participants
participants (2)
-
Eric Blake -
Michal Privoznik