On Mon, Feb 26, 2024 at 09:41:15AM -0500, Steven Sistare wrote: > On 2/26/2024 2:40 AM, Markus Armbruster wrote: > > Steve Sistare <steven.sist...@oracle.com> writes: > > > >> A small number of migration options are accessed by migration clients, > >> but to see them clients must include all of options.h, which is mostly > >> for migration core code. migrate_mode() in particular will be needed by > >> multiple clients. > >> > >> Refactor the option declarations so clients can see the necessary few via > >> misc.h, which already exports a portion of the client API. > >> > >> Signed-off-by: Steve Sistare <steven.sist...@oracle.com> > >> --- > >> I suggest that eventually we should define a single file migration/client.h > >> which exports everything needed by the simpler clients: blockers, > >> notifiers, > >> options, cpr, and state accessors. > >> --- > >> --- > >> hw/vfio/migration.c | 1 - > >> hw/virtio/virtio-balloon.c | 1 - > >> include/migration/misc.h | 1 + > >> include/migration/options-pub.h | 24 ++++++++++++++++++++++++ > >> migration/options.h | 6 +----- > > > > Unusual naming. We have zero headers named -pub.h or -public.h, and > > dozens named like -int.h or -internal.h. Please stick to the existing > > convention. > > In the spirit of minimizing changes, I went that route to avoid renaming the > existing migration/options.h and its references: > > 0 migration/block-dirty-bit 82 #include "options.h" > 1 migration/block.c 32 #include "options.h" > 2 migration/colo.c 37 #include "options.h" > 3 migration/migration-hmp-c 35 #include "options.h" > 4 migration/migration.c 68 #include "options.h" > 5 migration/multifd-zlib.c 21 #include "options.h" > 6 migration/multifd-zstd.c 21 #include "options.h" > 7 migration/multifd.c 29 #include "options.h" > 8 migration/options.c 30 #include "options.h" > 9 migration/postcopy-ram.c 40 #include "options.h" > a migration/qemu-file.c 33 #include "options.h" > b migration/ram-compress.c 37 #include "options.h" > c migration/ram.c 63 #include "options.h" > d migration/rdma.c 40 #include "options.h" > e migration/savevm.c 71 #include "options.h" > f migration/socket.c 30 #include "options.h" > g migration/tls.c 25 #include "options.h" > > But I take your point. > > Peter, which do you prefer?
>From statistics, "-internal.h" wins "-int.h": $ git grep "\-internal.h" | wc -l 135 $ git grep "\-int.h" | wc -l 3 > > A. rename: migration/options.h -> migration/options-internal.h > rename: include/migration/options-pub.h -> include/migration/options.h > > B. rename: include/migration/options.h -> include/migration/client-options.h > > I prefer B. If you prefer B, but want a different file name, please choose the > final name. Personally I don't have a strong opinion on the name. I'll see whether Markus has any comment. [and of course, I removed this patch from -staging queue to keep the discussion going..] -- Peter Xu
