[PATCH] virhostcpu: Fix build with clang and newest kernel headers
The most recent environment e.g. present in our Fedora Rawhide builds fail to build the tree with clang with the following error: ../src/util/virhostcpu.c:1291:25: error: field 'header' with variable sized type 'struct kvm_msrs' not at the end of a struct or class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end] struct kvm_msrs header; ^ The problem seems to be that clang doesn't like the new way the 'entries' field in struct kvm_msrs is declared. To work around the issue we can simply allocate the variable dynamically and use the 'entries' member as it was intended to to access the members. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virhostcpu.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 54d0166b85..c1e8dc8078 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1287,25 +1287,22 @@ virHostCPUGetMSRFromKVM(unsigned long index, uint64_t *result) { VIR_AUTOCLOSE fd = -1; - struct { - struct kvm_msrs header; - struct kvm_msr_entry entry; - } msr = { - .header = { .nmsrs = 1 }, - .entry = { .index = index }, - }; + g_autofree struct kvm_msrs *msr = g_malloc0(sizeof(struct kvm_msrs) + + sizeof(struct kvm_msr_entry)); + msr->nmsrs = 1; + msr->entries[0].index = index; if ((fd = open(KVM_DEVICE, O_RDONLY)) < 0) { virReportSystemError(errno, _("Unable to open %s"), KVM_DEVICE); return -1; } - if (ioctl(fd, KVM_GET_MSRS, &msr) < 0) { + if (ioctl(fd, KVM_GET_MSRS, msr) < 0) { VIR_DEBUG("Cannot get MSR 0x%lx from KVM", index); return 1; } - *result = msr.entry.data; + *result = msr->entries[0].data; return 0; } -- 2.37.1
On Tue, Aug 23, 2022 at 16:15:54 +0200, Peter Krempa wrote:
The most recent environment e.g. present in our Fedora Rawhide builds fail to build the tree with clang with the following error:
../src/util/virhostcpu.c:1291:25: error: field 'header' with variable sized type 'struct kvm_msrs' not at the end of a struct or class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end] struct kvm_msrs header; ^
The problem seems to be that clang doesn't like the new way the 'entries' field in struct kvm_msrs is declared.
To work around the issue we can simply allocate the variable dynamically and use the 'entries' member as it was intended to to access the members.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virhostcpu.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
On 8/23/22 16:15, Peter Krempa wrote:
The most recent environment e.g. present in our Fedora Rawhide builds fail to build the tree with clang with the following error:
../src/util/virhostcpu.c:1291:25: error: field 'header' with variable sized type 'struct kvm_msrs' not at the end of a struct or class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end] struct kvm_msrs header; ^
The problem seems to be that clang doesn't like the new way the 'entries' field in struct kvm_msrs is declared.
To work around the issue we can simply allocate the variable dynamically and use the 'entries' member as it was intended to to access the members.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virhostcpu.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
Yeah, since this code is heavily inspired by QEMU I wanted to wait a bit to see how QEMU deals with this because allocating those few bytes looked needless to me. But I guess there's no better solution. Anyway, I've raised this issue with QEMU here: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg03128.html Michal
On Tue, Aug 23, 2022 at 16:38:46 +0200, Michal Prívozník wrote:
On 8/23/22 16:15, Peter Krempa wrote:
The most recent environment e.g. present in our Fedora Rawhide builds fail to build the tree with clang with the following error:
../src/util/virhostcpu.c:1291:25: error: field 'header' with variable sized type 'struct kvm_msrs' not at the end of a struct or class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end] struct kvm_msrs header; ^
The problem seems to be that clang doesn't like the new way the 'entries' field in struct kvm_msrs is declared.
To work around the issue we can simply allocate the variable dynamically and use the 'entries' member as it was intended to to access the members.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virhostcpu.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
Yeah, since this code is heavily inspired by QEMU I wanted to wait a bit to see how QEMU deals with this because allocating those few bytes looked needless to me. But I guess there's no better solution. Anyway,
This is a usual way how we deal with such structs e.g. in virNetDevGetEthtoolGFeatures. Additionally now it uses the intended accessor declared in 'struct kvm_msrs'. Also we have A LOT places where we allocate short temp strings just to format something. I'd not worry about overhead too much.
On Tue, Aug 23, 2022 at 04:15:54PM +0200, Peter Krempa wrote:
The most recent environment e.g. present in our Fedora Rawhide builds fail to build the tree with clang with the following error:
../src/util/virhostcpu.c:1291:25: error: field 'header' with variable sized type 'struct kvm_msrs' not at the end of a struct or class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end] struct kvm_msrs header; ^
The problem seems to be that clang doesn't like the new way the 'entries' field in struct kvm_msrs is declared.
To work around the issue we can simply allocate the variable dynamically and use the 'entries' member as it was intended to to access the members.
We explicitly only support GCC and CLang, and intentionally rely on many GNU extensions to C. CLang is trying to be helpful to people who need fully portable C code, but we don't care about that. So why not just turn off the warning by adding -Wno-gnu-variable-sized-style-not-at-end better than making our code less understandable IMHO
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virhostcpu.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 54d0166b85..c1e8dc8078 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1287,25 +1287,22 @@ virHostCPUGetMSRFromKVM(unsigned long index, uint64_t *result) { VIR_AUTOCLOSE fd = -1; - struct { - struct kvm_msrs header; - struct kvm_msr_entry entry; - } msr = { - .header = { .nmsrs = 1 }, - .entry = { .index = index }, - }; + g_autofree struct kvm_msrs *msr = g_malloc0(sizeof(struct kvm_msrs) + + sizeof(struct kvm_msr_entry)); + msr->nmsrs = 1; + msr->entries[0].index = index;
if ((fd = open(KVM_DEVICE, O_RDONLY)) < 0) { virReportSystemError(errno, _("Unable to open %s"), KVM_DEVICE); return -1; }
- if (ioctl(fd, KVM_GET_MSRS, &msr) < 0) { + if (ioctl(fd, KVM_GET_MSRS, msr) < 0) { VIR_DEBUG("Cannot get MSR 0x%lx from KVM", index); return 1; }
- *result = msr.entry.data; + *result = msr->entries[0].data; return 0; }
-- 2.37.1
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 :|
On Tue, Aug 23, 2022 at 15:42:38 +0100, Daniel P. Berrangé wrote:
On Tue, Aug 23, 2022 at 04:15:54PM +0200, Peter Krempa wrote:
The most recent environment e.g. present in our Fedora Rawhide builds fail to build the tree with clang with the following error:
../src/util/virhostcpu.c:1291:25: error: field 'header' with variable sized type 'struct kvm_msrs' not at the end of a struct or class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end] struct kvm_msrs header; ^
The problem seems to be that clang doesn't like the new way the 'entries' field in struct kvm_msrs is declared.
To work around the issue we can simply allocate the variable dynamically and use the 'entries' member as it was intended to to access the members.
We explicitly only support GCC and CLang, and intentionally rely on many GNU extensions to C. CLang is trying to be helpful to people who need fully portable C code, but we don't care about that. So why not just turn off the warning by adding
-Wno-gnu-variable-sized-style-not-at-end
better than making our code less understandable IMHO
Well I guess mostly because the new code is actually easier to understand than the old one :-) Jirka
participants (4)
-
Daniel P. Berrangé -
Jiri Denemark -
Michal Prívozník -
Peter Krempa