[PATCH V2] migration: export fewer options

Steve Sistare posted 1 patch 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1708985769-280850-1-git-send-email-steven.sistare@oracle.com
Maintainers: Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, David Hildenbrand <david@redhat.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Hyman Huang <yong.huang@smartx.com>
There is a newer version of this series
hw/vfio/migration.c                |  1 -
hw/virtio/virtio-balloon.c         |  1 -
include/migration/client-options.h | 24 ++++++++++++++++++++++++
include/migration/misc.h           |  1 +
migration/options.h                |  6 +-----
system/dirtylimit.c                |  1 -
6 files changed, 26 insertions(+), 8 deletions(-)
create mode 100644 include/migration/client-options.h
[PATCH V2] migration: export fewer options
Posted by Steve Sistare 9 months ago
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>
---
Changes in V2:
  * renamed options-pub.h to client-options.h
---
---
 hw/vfio/migration.c                |  1 -
 hw/virtio/virtio-balloon.c         |  1 -
 include/migration/client-options.h | 24 ++++++++++++++++++++++++
 include/migration/misc.h           |  1 +
 migration/options.h                |  6 +-----
 system/dirtylimit.c                |  1 -
 6 files changed, 26 insertions(+), 8 deletions(-)
 create mode 100644 include/migration/client-options.h

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 50140ed..5d4a23c 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -18,7 +18,6 @@
 #include "sysemu/runstate.h"
 #include "hw/vfio/vfio-common.h"
 #include "migration/migration.h"
-#include "migration/options.h"
 #include "migration/savevm.h"
 #include "migration/vmstate.h"
 #include "migration/qemu-file.h"
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 89f853f..a59ff17 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -32,7 +32,6 @@
 #include "qemu/error-report.h"
 #include "migration/misc.h"
 #include "migration/migration.h"
-#include "migration/options.h"
 
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/virtio-access.h"
diff --git a/include/migration/client-options.h b/include/migration/client-options.h
new file mode 100644
index 0000000..887fea1
--- /dev/null
+++ b/include/migration/client-options.h
@@ -0,0 +1,24 @@
+/*
+ * QEMU public migration capabilities
+ *
+ * Copyright (c) 2012-2023 Red Hat Inc
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_MIGRATION_CLIENT_OPTIONS_H
+#define QEMU_MIGRATION_CLIENT_OPTIONS_H
+
+/* capabilities */
+
+bool migrate_background_snapshot(void);
+bool migrate_dirty_limit(void);
+bool migrate_postcopy_ram(void);
+bool migrate_switchover_ack(void);
+
+/* parameters */
+
+MigMode migrate_mode(void);
+
+#endif
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 5d1aa59..4c226a4 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -17,6 +17,7 @@
 #include "qemu/notify.h"
 #include "qapi/qapi-types-migration.h"
 #include "qapi/qapi-types-net.h"
+#include "migration/client-options.h"
 
 /* migration/ram.c */
 
diff --git a/migration/options.h b/migration/options.h
index 246c160..964ebdd 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -16,6 +16,7 @@
 
 #include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
+#include "migration/client-options.h"
 
 /* migration properties */
 
@@ -24,12 +25,10 @@ extern Property migration_properties[];
 /* capabilities */
 
 bool migrate_auto_converge(void);
-bool migrate_background_snapshot(void);
 bool migrate_block(void);
 bool migrate_colo(void);
 bool migrate_compress(void);
 bool migrate_dirty_bitmaps(void);
-bool migrate_dirty_limit(void);
 bool migrate_events(void);
 bool migrate_ignore_shared(void);
 bool migrate_late_block_activate(void);
@@ -37,11 +36,9 @@ bool migrate_multifd(void);
 bool migrate_pause_before_switchover(void);
 bool migrate_postcopy_blocktime(void);
 bool migrate_postcopy_preempt(void);
-bool migrate_postcopy_ram(void);
 bool migrate_rdma_pin_all(void);
 bool migrate_release_ram(void);
 bool migrate_return_path(void);
-bool migrate_switchover_ack(void);
 bool migrate_validate_uuid(void);
 bool migrate_xbzrle(void);
 bool migrate_zero_blocks(void);
@@ -83,7 +80,6 @@ uint8_t migrate_max_cpu_throttle(void);
 uint64_t migrate_max_bandwidth(void);
 uint64_t migrate_avail_switchover_bandwidth(void);
 uint64_t migrate_max_postcopy_bandwidth(void);
-MigMode migrate_mode(void);
 int migrate_multifd_channels(void);
 MultiFDCompression migrate_multifd_compression(void);
 int migrate_multifd_zlib_level(void);
diff --git a/system/dirtylimit.c b/system/dirtylimit.c
index b5607eb..774ff44 100644
--- a/system/dirtylimit.c
+++ b/system/dirtylimit.c
@@ -26,7 +26,6 @@
 #include "trace.h"
 #include "migration/misc.h"
 #include "migration/migration.h"
-#include "migration/options.h"
 
 /*
  * Dirtylimit stop working if dirty page rate error
-- 
1.8.3.1
Re: [PATCH V2] migration: export fewer options
Posted by Steven Sistare 9 months ago
Just a reminder, after our further discussion in the V1 thread, 
this patch is still what I propose, no updates needed.

Markus, I think Peter is looking for your blessing on the new
file name: include/migration/client-options.h.

- Steve

On 2/26/2024 5:16 PM, Steve Sistare wrote:
> 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>
> ---
> Changes in V2:
>   * renamed options-pub.h to client-options.h
> ---
> ---
>  hw/vfio/migration.c                |  1 -
>  hw/virtio/virtio-balloon.c         |  1 -
>  include/migration/client-options.h | 24 ++++++++++++++++++++++++
>  include/migration/misc.h           |  1 +
>  migration/options.h                |  6 +-----
>  system/dirtylimit.c                |  1 -
>  6 files changed, 26 insertions(+), 8 deletions(-)
>  create mode 100644 include/migration/client-options.h
> 
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index 50140ed..5d4a23c 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -18,7 +18,6 @@
>  #include "sysemu/runstate.h"
>  #include "hw/vfio/vfio-common.h"
>  #include "migration/migration.h"
> -#include "migration/options.h"
>  #include "migration/savevm.h"
>  #include "migration/vmstate.h"
>  #include "migration/qemu-file.h"
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 89f853f..a59ff17 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -32,7 +32,6 @@
>  #include "qemu/error-report.h"
>  #include "migration/misc.h"
>  #include "migration/migration.h"
> -#include "migration/options.h"
>  
>  #include "hw/virtio/virtio-bus.h"
>  #include "hw/virtio/virtio-access.h"
> diff --git a/include/migration/client-options.h b/include/migration/client-options.h
> new file mode 100644
> index 0000000..887fea1
> --- /dev/null
> +++ b/include/migration/client-options.h
> @@ -0,0 +1,24 @@
> +/*
> + * QEMU public migration capabilities
> + *
> + * Copyright (c) 2012-2023 Red Hat Inc
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef QEMU_MIGRATION_CLIENT_OPTIONS_H
> +#define QEMU_MIGRATION_CLIENT_OPTIONS_H
> +
> +/* capabilities */
> +
> +bool migrate_background_snapshot(void);
> +bool migrate_dirty_limit(void);
> +bool migrate_postcopy_ram(void);
> +bool migrate_switchover_ack(void);
> +
> +/* parameters */
> +
> +MigMode migrate_mode(void);
> +
> +#endif
> diff --git a/include/migration/misc.h b/include/migration/misc.h
> index 5d1aa59..4c226a4 100644
> --- a/include/migration/misc.h
> +++ b/include/migration/misc.h
> @@ -17,6 +17,7 @@
>  #include "qemu/notify.h"
>  #include "qapi/qapi-types-migration.h"
>  #include "qapi/qapi-types-net.h"
> +#include "migration/client-options.h"
>  
>  /* migration/ram.c */
>  
> diff --git a/migration/options.h b/migration/options.h
> index 246c160..964ebdd 100644
> --- a/migration/options.h
> +++ b/migration/options.h
> @@ -16,6 +16,7 @@
>  
>  #include "hw/qdev-properties.h"
>  #include "hw/qdev-properties-system.h"
> +#include "migration/client-options.h"
>  
>  /* migration properties */
>  
> @@ -24,12 +25,10 @@ extern Property migration_properties[];
>  /* capabilities */
>  
>  bool migrate_auto_converge(void);
> -bool migrate_background_snapshot(void);
>  bool migrate_block(void);
>  bool migrate_colo(void);
>  bool migrate_compress(void);
>  bool migrate_dirty_bitmaps(void);
> -bool migrate_dirty_limit(void);
>  bool migrate_events(void);
>  bool migrate_ignore_shared(void);
>  bool migrate_late_block_activate(void);
> @@ -37,11 +36,9 @@ bool migrate_multifd(void);
>  bool migrate_pause_before_switchover(void);
>  bool migrate_postcopy_blocktime(void);
>  bool migrate_postcopy_preempt(void);
> -bool migrate_postcopy_ram(void);
>  bool migrate_rdma_pin_all(void);
>  bool migrate_release_ram(void);
>  bool migrate_return_path(void);
> -bool migrate_switchover_ack(void);
>  bool migrate_validate_uuid(void);
>  bool migrate_xbzrle(void);
>  bool migrate_zero_blocks(void);
> @@ -83,7 +80,6 @@ uint8_t migrate_max_cpu_throttle(void);
>  uint64_t migrate_max_bandwidth(void);
>  uint64_t migrate_avail_switchover_bandwidth(void);
>  uint64_t migrate_max_postcopy_bandwidth(void);
> -MigMode migrate_mode(void);
>  int migrate_multifd_channels(void);
>  MultiFDCompression migrate_multifd_compression(void);
>  int migrate_multifd_zlib_level(void);
> diff --git a/system/dirtylimit.c b/system/dirtylimit.c
> index b5607eb..774ff44 100644
> --- a/system/dirtylimit.c
> +++ b/system/dirtylimit.c
> @@ -26,7 +26,6 @@
>  #include "trace.h"
>  #include "migration/misc.h"
>  #include "migration/migration.h"
> -#include "migration/options.h"
>  
>  /*
>   * Dirtylimit stop working if dirty page rate error
Re: [PATCH V2] migration: export fewer options
Posted by Markus Armbruster 9 months ago
Steven Sistare <steven.sistare@oracle.com> writes:

> Just a reminder, after our further discussion in the V1 thread, 
> this patch is still what I propose, no updates needed.
>
> Markus, I think Peter is looking for your blessing on the new
> file name: include/migration/client-options.h.

Not my preference, but no objection.
Re: [PATCH V2] migration: export fewer options
Posted by Peter Xu 8 months, 4 weeks ago
On Thu, Feb 29, 2024 at 07:03:36AM +0100, Markus Armbruster wrote:
> Steven Sistare <steven.sistare@oracle.com> writes:
> 
> > Just a reminder, after our further discussion in the V1 thread, 
> > this patch is still what I propose, no updates needed.
> >
> > Markus, I think Peter is looking for your blessing on the new
> > file name: include/migration/client-options.h.
> 
> Not my preference, but no objection.

There's yet one alternative, which is to put these exported option
functions into misc.h directly.  After all that's not so much, and misc.h
already hold random stuff from elsewhere.

Steve, would you repost this patch (with/without my above comment taken)
along with your other series with a rebase to migration-next?  It doesn't
apply there.  Re the other series: one nitpick comment on the last patch,
where you may consider splitting the removal of the unused 2 functions into
a standalone patch.  Other than that it looks good to me.

https://gitlab.com/peterx/qemu/-/tree/migration-next

Thanks,

-- 
Peter Xu
Re: [PATCH V2] migration: export fewer options
Posted by Steven Sistare 8 months, 2 weeks ago
On 3/1/2024 2:47 AM, Peter Xu wrote:
> On Thu, Feb 29, 2024 at 07:03:36AM +0100, Markus Armbruster wrote:
>> Steven Sistare <steven.sistare@oracle.com> writes:
>>
>>> Just a reminder, after our further discussion in the V1 thread,
>>> this patch is still what I propose, no updates needed.
>>>
>>> Markus, I think Peter is looking for your blessing on the new
>>> file name: include/migration/client-options.h.
>>
>> Not my preference, but no objection.
> 
> There's yet one alternative, which is to put these exported option
> functions into misc.h directly.  After all that's not so much, and misc.h
> already hold random stuff from elsewhere.
> 
> Steve, would you repost this patch (with/without my above comment taken)
> along with your other series with a rebase to migration-next?  It doesn't
> apply there.  Re the other series: one nitpick comment on the last patch,
> where you may consider splitting the removal of the unused 2 functions into
> a standalone patch.  Other than that it looks good to me.
> 
> https://gitlab.com/peterx/qemu/-/tree/migration-next

Both are rebased and reposted, thanks - steve