include/block/dirty-bitmap.h | 9 +- include/qemu/hbitmap.h | 95 +++-------- block/dirty-bitmap.c | 16 +- block/qcow2-bitmap.c | 15 +- nbd/server.c | 251 ++++++++++++++-------------- tests/test-hbitmap.c | 316 +++++++++++++---------------------- util/hbitmap.c | 134 +++++++++------ 7 files changed, 375 insertions(+), 461 deletions(-)
The following changes since commit 6e8a73e911f066527e775e04b98f31ebd19db600: Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2020-03-11 14:41:27 +0000) are available in the Git repository at: https://github.com/jnsnow/qemu.git tags/bitmaps-pull-request for you to fetch changes up to 34b456d485a4df3a88116fb5ef0c418f2f12990d: block/qcow2-bitmap: use bdrv_dirty_bitmap_next_dirty (2020-03-12 16:36:46 -0400) ---------------------------------------------------------------- Pull request ---------------------------------------------------------------- Vladimir Sementsov-Ogievskiy (10): hbitmap: assert that we don't create bitmap larger than INT64_MAX hbitmap: move hbitmap_iter_next_word to hbitmap.c hbitmap: unpublish hbitmap_iter_skip_words hbitmap: drop meta bitmaps as they are unused block/dirty-bitmap: switch _next_dirty_area and _next_zero to int64_t block/dirty-bitmap: add _next_dirty API block/dirty-bitmap: improve _next_dirty_area API nbd/server: introduce NBDExtentArray nbd/server: use bdrv_dirty_bitmap_next_dirty_area block/qcow2-bitmap: use bdrv_dirty_bitmap_next_dirty include/block/dirty-bitmap.h | 9 +- include/qemu/hbitmap.h | 95 +++-------- block/dirty-bitmap.c | 16 +- block/qcow2-bitmap.c | 15 +- nbd/server.c | 251 ++++++++++++++-------------- tests/test-hbitmap.c | 316 +++++++++++++---------------------- util/hbitmap.c | 134 +++++++++------ 7 files changed, 375 insertions(+), 461 deletions(-) -- 2.21.1
On Tue, 17 Mar 2020 at 04:38, John Snow <jsnow@redhat.com> wrote: > > The following changes since commit 6e8a73e911f066527e775e04b98f31ebd19db600: > > Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2020-03-11 14:41:27 +0000) > > are available in the Git repository at: > > https://github.com/jnsnow/qemu.git tags/bitmaps-pull-request > > for you to fetch changes up to 34b456d485a4df3a88116fb5ef0c418f2f12990d: > > block/qcow2-bitmap: use bdrv_dirty_bitmap_next_dirty (2020-03-12 16:36:46 -0400) > > ---------------------------------------------------------------- > Pull request > > --------------------------------------------------------------- Hi; this fails to compile with clang: /home/petmay01/linaro/qemu-for-merges/nbd/server.c:1937:1: error: unused function 'glib_listautoptr_cleanup_NBDExtentArray' [-Werror,-Wunused-function] G_DEFINE_AUTOPTR_CLEANUP_FUNC(NBDExtentArray, nbd_extent_array_free); ^ /usr/include/glib-2.0/glib/gmacros.h:462:22: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC' static inline void _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) (GList **_l) { g_list_free_full (*_l, (GDestroyNotify) func); } \ ^ /usr/include/glib-2.0/glib/gmacros.h:443:48: note: expanded from macro '_GLIB_AUTOPTR_LIST_FUNC_NAME' #define _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) glib_listautoptr_cleanup_##TypeName ^ <scratch space>:49:1: note: expanded from here glib_listautoptr_cleanup_NBDExtentArray ^ thanks -- PMM
On 3/17/20 9:00 AM, Peter Maydell wrote: > On Tue, 17 Mar 2020 at 04:38, John Snow <jsnow@redhat.com> wrote: >> >> block/qcow2-bitmap: use bdrv_dirty_bitmap_next_dirty (2020-03-12 16:36:46 -0400) >> >> ---------------------------------------------------------------- >> Pull request >> >> --------------------------------------------------------------- > > Hi; this fails to compile with clang: > > /home/petmay01/linaro/qemu-for-merges/nbd/server.c:1937:1: error: > unused function 'glib_listautoptr_cleanup_NBDExtentArray' > [-Werror,-Wunused-function] > G_DEFINE_AUTOPTR_CLEANUP_FUNC(NBDExtentArray, nbd_extent_array_free); > ^ > /usr/include/glib-2.0/glib/gmacros.h:462:22: note: expanded from macro > 'G_DEFINE_AUTOPTR_CLEANUP_FUNC' > static inline void _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) (GList > **_l) { g_list_free_full (*_l, (GDestroyNotify) func); } \ > ^ > /usr/include/glib-2.0/glib/gmacros.h:443:48: note: expanded from macro > '_GLIB_AUTOPTR_LIST_FUNC_NAME' > #define _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) > glib_listautoptr_cleanup_##TypeName > ^ > <scratch space>:49:1: note: expanded from here > glib_listautoptr_cleanup_NBDExtentArray > ^ Should we add -Wno-unused-function to CFLAGS when dealing with a version of clang that complains about that version of glib's headers? Is it fixed in a newer version of glib, where we could just backport a newer definition of G_DEFINE_AUTOPTR_CLEANUP_FUNC() that adds whatever annotations are needed to shut the compiler up? On IRC, danpb pointed me to libvirt's solution: https://libvirt.org/git/?p=libvirt.git;a=commit;h=44e7f029 Maybe we just write our own macro wrapper around G_DEFINE_AUTOPTR_CLEANUP_FUNC which takes care of adding necessary annotations and use that instead (and our macro name might be shorter...) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Tue, Mar 17, 2020 at 09:40:00AM -0500, Eric Blake wrote: > On 3/17/20 9:00 AM, Peter Maydell wrote: > > On Tue, 17 Mar 2020 at 04:38, John Snow <jsnow@redhat.com> wrote: > > > > > > > block/qcow2-bitmap: use bdrv_dirty_bitmap_next_dirty (2020-03-12 16:36:46 -0400) > > > > > > ---------------------------------------------------------------- > > > Pull request > > > > > > --------------------------------------------------------------- > > > > Hi; this fails to compile with clang: > > > > /home/petmay01/linaro/qemu-for-merges/nbd/server.c:1937:1: error: > > unused function 'glib_listautoptr_cleanup_NBDExtentArray' > > [-Werror,-Wunused-function] > > G_DEFINE_AUTOPTR_CLEANUP_FUNC(NBDExtentArray, nbd_extent_array_free); > > ^ > > /usr/include/glib-2.0/glib/gmacros.h:462:22: note: expanded from macro > > 'G_DEFINE_AUTOPTR_CLEANUP_FUNC' > > static inline void _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) (GList > > **_l) { g_list_free_full (*_l, (GDestroyNotify) func); } \ > > ^ > > /usr/include/glib-2.0/glib/gmacros.h:443:48: note: expanded from macro > > '_GLIB_AUTOPTR_LIST_FUNC_NAME' > > #define _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) > > glib_listautoptr_cleanup_##TypeName > > ^ > > <scratch space>:49:1: note: expanded from here > > glib_listautoptr_cleanup_NBDExtentArray > > ^ > > Should we add -Wno-unused-function to CFLAGS when dealing with a version of > clang that complains about that version of glib's headers? Is it fixed in a > newer version of glib, where we could just backport a newer definition of > G_DEFINE_AUTOPTR_CLEANUP_FUNC() that adds whatever annotations are needed to > shut the compiler up? > > On IRC, danpb pointed me to libvirt's solution: > https://libvirt.org/git/?p=libvirt.git;a=commit;h=44e7f029 > > Maybe we just write our own macro wrapper around > G_DEFINE_AUTOPTR_CLEANUP_FUNC which takes care of adding necessary > annotations and use that instead (and our macro name might be shorter...) My preference is to stick with regular glib functions/macros whereever practical, rather than inventing QEMU replacements which add a knowledge burden for contributors. That's why we moved to -Wno-unused-function in libvirt. I don't feel like -Wno-unused-function looses anything significant, as the GCC builds will still be reporting unused functions which will catch majority of cases. Possibly we could figure out a patch for glib upstream that uses pragma push/pop to squelch the warning ? They are quite receptive to patches IME. 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, 17 Mar 2020 at 14:57, Daniel P. Berrangé <berrange@redhat.com> wrote: > I don't feel like -Wno-unused-function looses anything significant, as > the GCC builds will still be reporting unused functions which will > catch majority of cases. The most interesting difference is that clang will catch unused static inline functions which gcc does not. thanks -- PMM
On Tue, Mar 17, 2020 at 03:00:48PM +0000, Peter Maydell wrote: > On Tue, 17 Mar 2020 at 14:57, Daniel P. Berrangé <berrange@redhat.com> wrote: > > I don't feel like -Wno-unused-function looses anything significant, as > > the GCC builds will still be reporting unused functions which will > > catch majority of cases. > > The most interesting difference is that clang will catch unused > static inline functions which gcc does not. That's mostly just about dead code cruft detection IIUC. That code won't make it into the binary if it isn't used. 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, 17 Mar 2020 at 15:05, Daniel P. Berrangé <berrange@redhat.com> wrote: > > On Tue, Mar 17, 2020 at 03:00:48PM +0000, Peter Maydell wrote: > > On Tue, 17 Mar 2020 at 14:57, Daniel P. Berrangé <berrange@redhat.com> wrote: > > > I don't feel like -Wno-unused-function looses anything significant, as > > > the GCC builds will still be reporting unused functions which will > > > catch majority of cases. > > > > The most interesting difference is that clang will catch unused > > static inline functions which gcc does not. > > That's mostly just about dead code cruft detection IIUC. That code won't > make it into the binary if it isn't used. Indeed, but it's nice to have the dead code cruft detection. You can always mark the function as __attribute__((unused)) if you really mean that it might be present but not used. thanks -- PMM
On Tue, Mar 17, 2020 at 03:07:34PM +0000, Peter Maydell wrote: > On Tue, 17 Mar 2020 at 15:05, Daniel P. Berrangé <berrange@redhat.com> wrote: > > > > On Tue, Mar 17, 2020 at 03:00:48PM +0000, Peter Maydell wrote: > > > On Tue, 17 Mar 2020 at 14:57, Daniel P. Berrangé <berrange@redhat.com> wrote: > > > > I don't feel like -Wno-unused-function looses anything significant, as > > > > the GCC builds will still be reporting unused functions which will > > > > catch majority of cases. > > > > > > The most interesting difference is that clang will catch unused > > > static inline functions which gcc does not. > > > > That's mostly just about dead code cruft detection IIUC. That code won't > > make it into the binary if it isn't used. > > Indeed, but it's nice to have the dead code cruft detection. You > can always mark the function as __attribute__((unused)) if you really > mean that it might be present but not used. The *BSDs seem to track latest glib pretty quickly. So if we got the unused attribute into upstream glib, we would probably have about 6-9 months before we get a build platform with the fixed glib included where we can conditionally re-enable the unused-function warning. 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 3/17/20 10:11 AM, Daniel P. Berrangé wrote: > On Tue, Mar 17, 2020 at 03:07:34PM +0000, Peter Maydell wrote: >> On Tue, 17 Mar 2020 at 15:05, Daniel P. Berrangé <berrange@redhat.com> wrote: >>> >>> On Tue, Mar 17, 2020 at 03:00:48PM +0000, Peter Maydell wrote: >>>> On Tue, 17 Mar 2020 at 14:57, Daniel P. Berrangé <berrange@redhat.com> wrote: >>>>> I don't feel like -Wno-unused-function looses anything significant, as >>>>> the GCC builds will still be reporting unused functions which will >>>>> catch majority of cases. >>>> >>>> The most interesting difference is that clang will catch unused >>>> static inline functions which gcc does not. >>> >>> That's mostly just about dead code cruft detection IIUC. That code won't >>> make it into the binary if it isn't used. >> >> Indeed, but it's nice to have the dead code cruft detection. You >> can always mark the function as __attribute__((unused)) if you really >> mean that it might be present but not used. > > The *BSDs seem to track latest glib pretty quickly. So if we got the > unused attribute into upstream glib, we would probably have about > 6-9 months before we get a build platform with the fixed glib included > where we can conditionally re-enable the unused-function warning. Looks like glib commit https://github.com/GNOME/glib/commit/b41bff1f (2.57.2) added G_GNUC_UNUSED to all of the functions declared during G_DEFINE_AUTOPTR_CLEANUP_FUNC. Which version of glib is on the NetBSD machine? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On 3/17/20 9:00 AM, Peter Maydell wrote: >> block/qcow2-bitmap: use bdrv_dirty_bitmap_next_dirty (2020-03-12 16:36:46 -0400) >> >> ---------------------------------------------------------------- >> Pull request >> >> --------------------------------------------------------------- > > Hi; this fails to compile with clang: > As pointed out here, my recommendation is for John to send a v2 pull request with one more patch added: https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg05969.html -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
© 2016 - 2024 Red Hat, Inc.