Re: [PATCH 35/40] libxl: convert libxlMigrationDstArgs to GObject
On Mon, May 18, 2020 at 05:16:29PM +0200, Rafael Fonseca wrote:
On Mon, May 18, 2020 at 3:28 PM Rafael Fonseca
<r4f4rfs(a)gmail.com> wrote:
>
> On Mon, May 18, 2020 at 11:31 AM Daniel P. Berrangé <berrange(a)redhat.com>
wrote:
> >
> > On Sun, May 17, 2020 at 11:03:57PM +0200, Rafael Fonseca wrote:
> > > On Fri, May 15, 2020 at 5:24 PM Daniel P. Berrangé
<berrange(a)redhat.com> wrote:
> > > > Since we're using GitLab now, if you just fork the libvirt repo
> > > > and push a branch to your gitlab fork, it will run CI which will
> > > > check for these mistakes.
> > >
> > > Good to know. I did that and the CI is only failing now in the
> > > cross-compilation phase with 32-bit systems:
> > >
> > > ../../src/util/virstoragefile.h: In function
'VIR_STORAGE_SOURCE':
> > >
/usr/i686-w64-mingw32/sys-root/mingw/include/glib-2.0/gobject/gtype.h:2301:6:
> > > error: cast increases required alignment of target type
> > > [-Werror=cast-align]
> > > 2301 | ((ct*) g_type_check_instance_cast ((GTypeInstance*) ip, gt))
> > > | ^
> > >
/usr/i686-w64-mingw32/sys-root/mingw/include/glib-2.0/gobject/gtype.h:484:66:
> > > note: in expansion of macro '_G_TYPE_CIC'
> > > 484 | #define G_TYPE_CHECK_INSTANCE_CAST(instance, g_type, c_type)
> > > (_G_TYPE_CIC ((instance), (g_type), c_type))
> > > | ^~~~~~~~~~~
> > >
/usr/i686-w64-mingw32/sys-root/mingw/include/glib-2.0/gobject/gtype.h:1412:12:
> > > note: in expansion of macro 'G_TYPE_CHECK_INSTANCE_CAST'
> > > 1412 | return G_TYPE_CHECK_INSTANCE_CAST (ptr,
> > > module_obj_name##_get_type (), ModuleObjName); } \
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~
> > > ../../src/util/virstoragefile.h:390:1: note: in expansion of macro
> > > 'G_DECLARE_FINAL_TYPE'
> > > 390 | G_DECLARE_FINAL_TYPE(virStorageSource, vir_storage_source, VIR,
> > > STORAGE_SOURCE, GObject);
> > > | ^~~~~~~~~~~~~~~~~~~~
> > >
> > > Any ideas on how to solve that?
> >
> > This is odd and I can't tell why. Assyuming only virStorageSource shows
> > the issue though, there must be something in the way the struct is
> > arranged that causes alignment issues.
>
> I disabled the virStorageSource patch and tried again. On armv7l it
> failed with the same error but in a different part:
>
> ../../src/qemu/qemu_conf.h: In function 'VIR_QEMU_DRIVER_CONFIG':
Just to complete the information, I also disabled the QEmuDriverConfig
patch and the CI finished successfully in armv7l. So it's something in
virStorageSource and QemuDriverConfig, either the structs themselves
or something missing from my changes.
You're not doing anything wrong. The compiler doesn't have enough info to
realize that its warning is wrong.
We have a "GObject *" which requires 4 byte alignment, and are trying to
cast to "virStorageSource *" or "QemuDriverConfig *" which require 8
byte
alignment.
The compiler is pessimistic and has to consider that the original memory
for the "GObject *" is only aligned to 4 byte boundary, and thus a bad
cast would result if we cast to "virStorageSource *".
What the compiler does not know is that any "GObject *" we have will always
have been allocated using g_object_new, which delegates to g_slice_new,
which uses malloc or a slab allocator. Both malloc & the glib slab allocator
guarantee that all memory they allocate is aligned to 8 byte boundaries on
32-bit.
So the only way the cast from GObject * -> virStorageSource * could be a
bad cast is if we allocated the GObject on the stack but that'll never
happen.
So the cast alignment warnings are bogus.
The difficulty is that I don't see a nice way to eliminate the warnings.
Options I see include:
1 Turn off -Wcast-align entirely
2 Modify GLib to add attribute(align(8)) to GObject
3 Modify GLib to use Pragma push/pop to selectively disable -Wcast-align
I think (2) is an ABI change is probably not possible. Possibly we have
todo (1) and then also do (3) so we can revert (1) in future.
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 :|