[PATCH 35/40] libxl: convert libxlMigrationDstArgs to GObject

Daniel P. Berrangé berrange at redhat.com
Thu Jun 4 09:15:57 UTC 2020


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 at gmail.com> wrote:
> >
> > On Mon, May 18, 2020 at 11:31 AM Daniel P. Berrangé <berrange at 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 at 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 :|




More information about the libvir-list mailing list