[PATCH] util:hostcpu: Report physical address size based on Architecture

Narayana Murty N posted 1 patch 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20231004055841.320816-1-nnmlinux@linux.ibm.com
src/util/virarch.h    | 3 +++
src/util/virhostcpu.c | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
[PATCH] util:hostcpu: Report physical address size based on Architecture
Posted by Narayana Murty N 7 months ago
The function virHostCPUGetPhysAddrSize was introduced with commit be1b7d5b18e
fails on architectures other than x86 and SuperH. The commit 8417c1394cd4d
fixed the issue only for s390 but the problem is still seen on other
architectures like ppc which does not report Physical address size in their
cpuinfo output.

command:
systemctl restart libvirtd.service
Output :
<snip>
dnsmasq[2377]: read /var/lib/libvirt/dnsmasq/default.addnhosts - 0
addresses
dnsmasq-dhcp[2377]: read /var/lib/libvirt/dnsmasq/default.hostsfile
libvirtd[3163]: libvirt version: 9.8.0
libvirtd[3163]: hostname: xxxxxxxxxx
libvirtd[3163]: internal error: Missing or invalid CPU address size in
/proc/cpuinfo
 libvirtd.service: Deactivated successfully.
 </snip>

This patch fixes this issue by returning the size=0 for architectures
other than x86 and SuperH.

Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
---
 src/util/virarch.h    | 3 +++
 src/util/virhostcpu.c | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/util/virarch.h b/src/util/virarch.h
index 81b1b27a57..747f77c48e 100644
--- a/src/util/virarch.h
+++ b/src/util/virarch.h
@@ -103,6 +103,9 @@ typedef enum {
 #define ARCH_IS_MIPS64(arch) ((arch) == VIR_ARCH_MIPS64 ||\
                               (arch) == VIR_ARCH_MIPS64EL)
 
+#define ARCH_IS_SH4(arch) ((arch) == VIR_ARCH_SH4 ||\
+                           (arch) == VIR_ARCH_SH4EB)
+
 typedef enum {
     VIR_ARCH_LITTLE_ENDIAN,
     VIR_ARCH_BIG_ENDIAN,
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index 0389012ef7..4027547e1e 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -1650,7 +1650,7 @@ virHostCPUGetPhysAddrSize(const virArch hostArch,
 {
     g_autoptr(FILE) cpuinfo = NULL;
 
-    if (ARCH_IS_S390(hostArch)) {
+    if (!(ARCH_IS_X86(hostArch) || ARCH_IS_SH4(hostArch))) {
         /* Ensure size is set to 0 as physical address size is unknown */
         *size = 0;
         return 0;
-- 
2.39.2
Re: [PATCH] util:hostcpu: Report physical address size based on Architecture
Posted by Michal Prívozník 6 months, 2 weeks ago
On 10/4/23 07:58, Narayana Murty N wrote:
> The function virHostCPUGetPhysAddrSize was introduced with commit be1b7d5b18e
> fails on architectures other than x86 and SuperH. The commit 8417c1394cd4d
> fixed the issue only for s390 but the problem is still seen on other
> architectures like ppc which does not report Physical address size in their
> cpuinfo output.
> 
> command:
> systemctl restart libvirtd.service
> Output :
> <snip>
> dnsmasq[2377]: read /var/lib/libvirt/dnsmasq/default.addnhosts - 0
> addresses
> dnsmasq-dhcp[2377]: read /var/lib/libvirt/dnsmasq/default.hostsfile
> libvirtd[3163]: libvirt version: 9.8.0
> libvirtd[3163]: hostname: xxxxxxxxxx
> libvirtd[3163]: internal error: Missing or invalid CPU address size in
> /proc/cpuinfo
>  libvirtd.service: Deactivated successfully.
>  </snip>
> 
> This patch fixes this issue by returning the size=0 for architectures
> other than x86 and SuperH.
> 
> Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
> ---
>  src/util/virarch.h    | 3 +++
>  src/util/virhostcpu.c | 2 +-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 

Whoa, I had no idea that SH is still alive (an well?).

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and pushed. Congratulations on your first libvirt contribution.

Michal
Re: [PATCH] util:hostcpu: Report physical address size based on Architecture
Posted by Andrea Bolognani 6 months, 1 week ago
On Fri, Oct 20, 2023 at 12:48:26PM +0200, Michal Prívozník wrote:
> On 10/4/23 07:58, Narayana Murty N wrote:
> > This patch fixes this issue by returning the size=0 for architectures
> > other than x86 and SuperH.
>
> Whoa, I had no idea that SH is still alive (an well?).

As of a few months ago, Debian builds libvirt on SuperH:

  https://buildd.debian.org/status/logs.php?pkg=libvirt&arch=sh4

The QEMU driver is disabled, since there is no matching qemu-system
binary, but the LXC driver should (at least theoretically) work :)

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH] util:hostcpu: Report physical address size based on Architecture
Posted by Daniel P. Berrangé 6 months, 1 week ago
On Wed, Oct 25, 2023 at 09:12:32AM -0700, Andrea Bolognani wrote:
> On Fri, Oct 20, 2023 at 12:48:26PM +0200, Michal Prívozník wrote:
> > On 10/4/23 07:58, Narayana Murty N wrote:
> > > This patch fixes this issue by returning the size=0 for architectures
> > > other than x86 and SuperH.
> >
> > Whoa, I had no idea that SH is still alive (an well?).
> 
> As of a few months ago, Debian builds libvirt on SuperH:
> 
>   https://buildd.debian.org/status/logs.php?pkg=libvirt&arch=sh4
> 
> The QEMU driver is disabled, since there is no matching qemu-system
> binary, but the LXC driver should (at least theoretically) work :)

Maybe I misinterpret what you're saying here, but the lack of a
qemu-system-sh binary shouldn't be a reason to disable the QEMU
driver, as the user can use non-native qemu-system binaries.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] util:hostcpu: Report physical address size based on Architecture
Posted by Andrea Bolognani 6 months, 1 week ago
On Wed, Oct 25, 2023 at 05:27:07PM +0100, Daniel P. Berrangé wrote:
> On Wed, Oct 25, 2023 at 09:12:32AM -0700, Andrea Bolognani wrote:
> > On Fri, Oct 20, 2023 at 12:48:26PM +0200, Michal Prívozník wrote:
> > > On 10/4/23 07:58, Narayana Murty N wrote:
> > > > This patch fixes this issue by returning the size=0 for architectures
> > > > other than x86 and SuperH.
> > >
> > > Whoa, I had no idea that SH is still alive (an well?).
> >
> > As of a few months ago, Debian builds libvirt on SuperH:
> >
> >   https://buildd.debian.org/status/logs.php?pkg=libvirt&arch=sh4
> >
> > The QEMU driver is disabled, since there is no matching qemu-system
> > binary, but the LXC driver should (at least theoretically) work :)
>
> Maybe I misinterpret what you're saying here, but the lack of a
> qemu-system-sh binary shouldn't be a reason to disable the QEMU
> driver, as the user can use non-native qemu-system binaries.

Yeah, sorry, my use of "matching" here was misleading.

What I wanted to say is that none of the qemu-system binaries are
built on sh4[1], so there is no way for the QEMU driver to do
anything useful.


[1] https://salsa.debian.org/qemu-team/qemu/-/blob/6d78618959cfae452801b596074dc67c55e5b087/debian/rules#L39-52
-- 
Andrea Bolognani / Red Hat / Virtualization