[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH V1] migration: export fewer options
From: |
Peter Xu |
Subject: |
Re: [PATCH V1] migration: export fewer options |
Date: |
Tue, 27 Feb 2024 16:08:25 +0800 |
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.sistare@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.sistare@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