[PATCH v3 0/5] Guestperf: miscellaneous refinement and enrichment

[PATCH v3 0/5] Guestperf: miscellaneous refinement and enrichment

[yong.huang]

From: Hyman Huang <yong.huang@smartx.com>

v3:
1. Remove the two redundant assignments in [PATCH v2 2/5] suggested by Daniel 

Please review, thanks
Yong

v2:
1. Update the MAINTAINERS section suggested by Fabiano Rosas 
2. Ensure the dependencies when build the initrd-stress.img suggested by Daniel
3. Fix some bugs

v1:
The previous patchset:
https://lore.kernel.org/qemu-devel/cover.1722957352.git.yong.huang@smartx.com/
does not made the necessary changes and tests for the upstream version.

This patchset works for that:
1. Move the guestperf to scripts directory suggested by Fabiano Rosas
2. Make initrd-stress.img built by default suggested by Fabiano Rosas
3. Make the necessary changes to adapt the latest multifd behavior
4. A nitpick for multifd migration
5. Support multifd compression option

Hyman Huang (5):
  tests/migration: Move the guestperf tool to scripts directory
  tests/migration: Make initrd-stress.img built by default
  guestperf: Support deferred migration for multifd
  guestperf: Nitpick the inconsistent parameters
  guestperf: Introduce multifd compression option

 MAINTAINERS                                   |  5 +++
 .../migration/guestperf-batch.py              |  0
 .../migration/guestperf-plot.py               |  0
 {tests => scripts}/migration/guestperf.py     |  0
 .../migration/guestperf/__init__.py           |  0
 .../migration/guestperf/comparison.py         | 15 ++++++++-
 .../migration/guestperf/engine.py             | 33 ++++++++++++++++---
 .../migration/guestperf/hardware.py           |  0
 .../migration/guestperf/plot.py               |  0
 .../migration/guestperf/progress.py           |  0
 .../migration/guestperf/report.py             |  0
 .../migration/guestperf/scenario.py           |  7 ++--
 .../migration/guestperf/shell.py              |  3 ++
 .../migration/guestperf/timings.py            |  0
 tests/migration/meson.build                   | 30 +++++++++--------
 15 files changed, 73 insertions(+), 20 deletions(-)
 rename {tests => scripts}/migration/guestperf-batch.py (100%)
 rename {tests => scripts}/migration/guestperf-plot.py (100%)
 rename {tests => scripts}/migration/guestperf.py (100%)
 rename {tests => scripts}/migration/guestperf/__init__.py (100%)
 rename {tests => scripts}/migration/guestperf/comparison.py (89%)
 rename {tests => scripts}/migration/guestperf/engine.py (93%)
 rename {tests => scripts}/migration/guestperf/hardware.py (100%)
 rename {tests => scripts}/migration/guestperf/plot.py (100%)
 rename {tests => scripts}/migration/guestperf/progress.py (100%)
 rename {tests => scripts}/migration/guestperf/report.py (100%)
 rename {tests => scripts}/migration/guestperf/scenario.py (93%)
 rename {tests => scripts}/migration/guestperf/shell.py (98%)
 rename {tests => scripts}/migration/guestperf/timings.py (100%)

-- 
2.27.0

[PATCH v3 1/5] tests/migration: Move the guestperf tool to scripts directory

[yong.huang]

From: Hyman Huang <yong.huang@smartx.com>

Guestperf was designed to test the performance of migration,
with a loose connection to the fundamental test cases of QEMU.

To improve the repository's structure, move it to the scripts
directory.

Add myself as a maintainer for the guestperf so that I can
help to fix bugs.

Signed-off-by: Hyman Huang <yong.huang@smartx.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 MAINTAINERS                                          | 5 +++++
 {tests => scripts}/migration/guestperf-batch.py      | 0
 {tests => scripts}/migration/guestperf-plot.py       | 0
 {tests => scripts}/migration/guestperf.py            | 0
 {tests => scripts}/migration/guestperf/__init__.py   | 0
 {tests => scripts}/migration/guestperf/comparison.py | 0
 {tests => scripts}/migration/guestperf/engine.py     | 0
 {tests => scripts}/migration/guestperf/hardware.py   | 0
 {tests => scripts}/migration/guestperf/plot.py       | 0
 {tests => scripts}/migration/guestperf/progress.py   | 0
 {tests => scripts}/migration/guestperf/report.py     | 0
 {tests => scripts}/migration/guestperf/scenario.py   | 0
 {tests => scripts}/migration/guestperf/shell.py      | 0
 {tests => scripts}/migration/guestperf/timings.py    | 0
 14 files changed, 5 insertions(+)
 rename {tests => scripts}/migration/guestperf-batch.py (100%)
 rename {tests => scripts}/migration/guestperf-plot.py (100%)
 rename {tests => scripts}/migration/guestperf.py (100%)
 rename {tests => scripts}/migration/guestperf/__init__.py (100%)
 rename {tests => scripts}/migration/guestperf/comparison.py (100%)
 rename {tests => scripts}/migration/guestperf/engine.py (100%)
 rename {tests => scripts}/migration/guestperf/hardware.py (100%)
 rename {tests => scripts}/migration/guestperf/plot.py (100%)
 rename {tests => scripts}/migration/guestperf/progress.py (100%)
 rename {tests => scripts}/migration/guestperf/report.py (100%)
 rename {tests => scripts}/migration/guestperf/scenario.py (100%)
 rename {tests => scripts}/migration/guestperf/shell.py (100%)
 rename {tests => scripts}/migration/guestperf/timings.py (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index c21d6a2f9e..c7938c397c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3403,6 +3403,11 @@ F: migration/dirtyrate.h
 F: include/sysemu/dirtyrate.h
 F: docs/devel/migration/dirty-limit.rst
 
+Migration performance test tool
+M: Hyman Huang <yong.huang@smartx.com>
+S: Maintained
+F: scripts/migration/guestperf*
+
 Detached LUKS header
 M: Hyman Huang <yong.huang@smartx.com>
 S: Maintained
diff --git a/tests/migration/guestperf-batch.py b/scripts/migration/guestperf-batch.py
similarity index 100%
rename from tests/migration/guestperf-batch.py
rename to scripts/migration/guestperf-batch.py
diff --git a/tests/migration/guestperf-plot.py b/scripts/migration/guestperf-plot.py
similarity index 100%
rename from tests/migration/guestperf-plot.py
rename to scripts/migration/guestperf-plot.py
diff --git a/tests/migration/guestperf.py b/scripts/migration/guestperf.py
similarity index 100%
rename from tests/migration/guestperf.py
rename to scripts/migration/guestperf.py
diff --git a/tests/migration/guestperf/__init__.py b/scripts/migration/guestperf/__init__.py
similarity index 100%
rename from tests/migration/guestperf/__init__.py
rename to scripts/migration/guestperf/__init__.py
diff --git a/tests/migration/guestperf/comparison.py b/scripts/migration/guestperf/comparison.py
similarity index 100%
rename from tests/migration/guestperf/comparison.py
rename to scripts/migration/guestperf/comparison.py
diff --git a/tests/migration/guestperf/engine.py b/scripts/migration/guestperf/engine.py
similarity index 100%
rename from tests/migration/guestperf/engine.py
rename to scripts/migration/guestperf/engine.py
diff --git a/tests/migration/guestperf/hardware.py b/scripts/migration/guestperf/hardware.py
similarity index 100%
rename from tests/migration/guestperf/hardware.py
rename to scripts/migration/guestperf/hardware.py
diff --git a/tests/migration/guestperf/plot.py b/scripts/migration/guestperf/plot.py
similarity index 100%
rename from tests/migration/guestperf/plot.py
rename to scripts/migration/guestperf/plot.py
diff --git a/tests/migration/guestperf/progress.py b/scripts/migration/guestperf/progress.py
similarity index 100%
rename from tests/migration/guestperf/progress.py
rename to scripts/migration/guestperf/progress.py
diff --git a/tests/migration/guestperf/report.py b/scripts/migration/guestperf/report.py
similarity index 100%
rename from tests/migration/guestperf/report.py
rename to scripts/migration/guestperf/report.py
diff --git a/tests/migration/guestperf/scenario.py b/scripts/migration/guestperf/scenario.py
similarity index 100%
rename from tests/migration/guestperf/scenario.py
rename to scripts/migration/guestperf/scenario.py
diff --git a/tests/migration/guestperf/shell.py b/scripts/migration/guestperf/shell.py
similarity index 100%
rename from tests/migration/guestperf/shell.py
rename to scripts/migration/guestperf/shell.py
diff --git a/tests/migration/guestperf/timings.py b/scripts/migration/guestperf/timings.py
similarity index 100%
rename from tests/migration/guestperf/timings.py
rename to scripts/migration/guestperf/timings.py
-- 
2.27.0

[PATCH v3 2/5] tests/migration: Make initrd-stress.img built by default

[yong.huang]

From: Hyman Huang <yong.huang@smartx.com>

The initrd-stress.img was compiled by specifying the target,
to make it easier for developers to play the guestperf tool,
make it built when dependencies suffices.

Signed-off-by: Hyman Huang <yong.huang@smartx.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/migration/meson.build | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/tests/migration/meson.build b/tests/migration/meson.build
index a91aa61c65..e10acbe317 100644
--- a/tests/migration/meson.build
+++ b/tests/migration/meson.build
@@ -2,17 +2,21 @@ sysprof = dependency('sysprof-capture-4', method: 'pkg-config', required: false)
 glib_static = dependency('glib-2.0', version: glib_req_ver, required: false,
                          method: 'pkg-config', static: true)
 
-stress = executable(
-  'stress',
-  files('stress.c'),
-  dependencies: [glib_static, sysprof],
-  link_args: ['-static'],
-  build_by_default: false,
-)
 
-custom_target(
-  'initrd-stress.img',
-  output: 'initrd-stress.img',
-  input: stress,
-  command: [find_program('initrd-stress.sh'), '@OUTPUT@', '@INPUT@']
-)
+if host_os == 'linux' and sysprof.found() and glib_static.found()
+    stress = executable(
+      'stress',
+      files('stress.c'),
+      dependencies: [glib_static, sysprof],
+      link_args: ['-static'],
+    )
+
+    custom_target(
+      'initrd-stress.img',
+      output: 'initrd-stress.img',
+      input: stress,
+      command: [find_program('initrd-stress.sh'), '@OUTPUT@', '@INPUT@'],
+      build_by_default: true,
+      depends: [stress],
+    )
+endif
-- 
2.27.0

[PATCH v3 3/5] guestperf: Support deferred migration for multifd

[yong.huang]

From: Hyman Huang <yong.huang@smartx.com>

The way to enable multifd migration has been changed by commit,
82137e6c8c (migration: enforce multifd and postcopy preempt to
be set before incoming), and guestperf has not made the
necessary changes. If multifd migration had been enabled in the
previous manner, the following error would have occurred:
Multifd must be set before incoming starts

Supporting deferred migration will fix it.

Signed-off-by: Hyman Huang <yong.huang@smartx.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 scripts/migration/guestperf/engine.py | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/scripts/migration/guestperf/engine.py b/scripts/migration/guestperf/engine.py
index 608d7270f6..4b15322e8d 100644
--- a/scripts/migration/guestperf/engine.py
+++ b/scripts/migration/guestperf/engine.py
@@ -106,7 +106,8 @@ def _migrate_progress(self, vm):
             info.get("dirty-limit-ring-full-time", 0),
         )
 
-    def _migrate(self, hardware, scenario, src, dst, connect_uri):
+    def _migrate(self, hardware, scenario, src,
+                 dst, connect_uri, defer_migrate):
         src_qemu_time = []
         src_vcpu_time = []
         src_pid = src.get_pid()
@@ -220,6 +221,8 @@ def _migrate(self, hardware, scenario, src, dst, connect_uri):
             resp = src.cmd("migrate-set-parameters",
                            vcpu_dirty_limit=scenario._vcpu_dirty_limit)
 
+        if defer_migrate:
+            resp = dst.cmd("migrate-incoming", uri=connect_uri)
         resp = src.cmd("migrate", uri=connect_uri)
 
         post_copy = False
@@ -373,11 +376,14 @@ def _get_common_args(self, hardware, tunnelled=False):
     def _get_src_args(self, hardware):
         return self._get_common_args(hardware)
 
-    def _get_dst_args(self, hardware, uri):
+    def _get_dst_args(self, hardware, uri, defer_migrate):
         tunnelled = False
         if self._dst_host != "localhost":
             tunnelled = True
         argv = self._get_common_args(hardware, tunnelled)
+
+        if defer_migrate:
+            return argv + ["-incoming", "defer"]
         return argv + ["-incoming", uri]
 
     @staticmethod
@@ -424,6 +430,7 @@ def _get_timings(self, vm):
 
     def run(self, hardware, scenario, result_dir=os.getcwd()):
         abs_result_dir = os.path.join(result_dir, scenario._name)
+        defer_migrate = False
 
         if self._transport == "tcp":
             uri = "tcp:%s:9000" % self._dst_host
@@ -439,6 +446,9 @@ def run(self, hardware, scenario, result_dir=os.getcwd()):
             except:
                 pass
 
+        if scenario._multifd:
+            defer_migrate = True
+
         if self._dst_host != "localhost":
             dstmonaddr = ("localhost", 9001)
         else:
@@ -452,7 +462,7 @@ def run(self, hardware, scenario, result_dir=os.getcwd()):
                           monitor_address=srcmonaddr)
 
         dst = QEMUMachine(self._binary,
-                          args=self._get_dst_args(hardware, uri),
+                          args=self._get_dst_args(hardware, uri, defer_migrate),
                           wrapper=self._get_dst_wrapper(hardware),
                           name="qemu-dst-%d" % os.getpid(),
                           monitor_address=dstmonaddr)
@@ -461,7 +471,8 @@ def run(self, hardware, scenario, result_dir=os.getcwd()):
             src.launch()
             dst.launch()
 
-            ret = self._migrate(hardware, scenario, src, dst, uri)
+            ret = self._migrate(hardware, scenario, src,
+                                dst, uri, defer_migrate)
             progress_history = ret[0]
             qemu_timings = ret[1]
             vcpu_timings = ret[2]
-- 
2.27.0

[PATCH v3 4/5] guestperf: Nitpick the inconsistent parameters

[yong.huang]

From: Hyman Huang <yong.huang@smartx.com>

Signed-off-by: Hyman Huang <yong.huang@smartx.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 scripts/migration/guestperf/comparison.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/migration/guestperf/comparison.py b/scripts/migration/guestperf/comparison.py
index 42cc0372d1..40e9d2eb1d 100644
--- a/scripts/migration/guestperf/comparison.py
+++ b/scripts/migration/guestperf/comparison.py
@@ -127,7 +127,7 @@ def __init__(self, name, scenarios):
     # varying numbers of channels
     Comparison("compr-multifd", scenarios = [
         Scenario("compr-multifd-channels-4",
-                 multifd=True, multifd_channels=2),
+                 multifd=True, multifd_channels=4),
         Scenario("compr-multifd-channels-8",
                  multifd=True, multifd_channels=8),
         Scenario("compr-multifd-channels-32",
-- 
2.27.0

[PATCH v3 5/5] guestperf: Introduce multifd compression option

[yong.huang]

From: Hyman Huang <yong.huang@smartx.com>

Guestperf tool does not cover the multifd compression option
currently, it is worth supporting so that developers can
analysis the migration performance with different
compression algorithms.

Multifd support 4 compression algorithms currently:
zlib, zstd, qpl, uadk

To request that multifd with the specified compression
algorithm such as zlib:
$ ./scripts/migration/guestperf.py \
    --multifd --multifd-channels 4 --multifd-compression zlib \
    --output output.json

To run the entire standardized set of multifd compression
comparisons, with unix migration:
$ ./scripts/migration/guestperf-batch.py \
    --dst-host localhost --transport unix \
    --filter compr-multifd-compression* --output outputdir

Signed-off-by: Hyman Huang <yong.huang@smartx.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 scripts/migration/guestperf/comparison.py | 13 +++++++++++++
 scripts/migration/guestperf/engine.py     | 14 ++++++++++++++
 scripts/migration/guestperf/scenario.py   |  7 +++++--
 scripts/migration/guestperf/shell.py      |  3 +++
 4 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/scripts/migration/guestperf/comparison.py b/scripts/migration/guestperf/comparison.py
index 40e9d2eb1d..dee3ac25e4 100644
--- a/scripts/migration/guestperf/comparison.py
+++ b/scripts/migration/guestperf/comparison.py
@@ -158,4 +158,17 @@ def __init__(self, name, scenarios):
         Scenario("compr-dirty-limit-50MB",
                  dirty_limit=True, vcpu_dirty_limit=50),
     ]),
+
+    # Looking at effect of multifd with
+    # different compression algorithms
+    Comparison("compr-multifd-compression", scenarios = [
+        Scenario("compr-multifd-compression-zlib",
+                 multifd=True, multifd_channels=2, multifd_compression="zlib"),
+        Scenario("compr-multifd-compression-zstd",
+                 multifd=True, multifd_channels=2, multifd_compression="zstd"),
+        Scenario("compr-multifd-compression-qpl",
+                 multifd=True, multifd_channels=2, multifd_compression="qpl"),
+        Scenario("compr-multifd-compression-uadk",
+                 multifd=True, multifd_channels=2, multifd_compression="uadk"),
+    ]),
 ]
diff --git a/scripts/migration/guestperf/engine.py b/scripts/migration/guestperf/engine.py
index 4b15322e8d..e11f6a8496 100644
--- a/scripts/migration/guestperf/engine.py
+++ b/scripts/migration/guestperf/engine.py
@@ -31,6 +31,8 @@
                              '..', '..', '..', 'python'))
 from qemu.machine import QEMUMachine
 
+# multifd supported compression algorithms
+MULTIFD_CMP_ALGS = ("zlib", "zstd", "qpl", "uadk")
 
 class Engine(object):
 
@@ -191,6 +193,12 @@ def _migrate(self, hardware, scenario, src,
                                scenario._compression_xbzrle_cache))
 
         if scenario._multifd:
+            if (scenario._multifd_compression and
+                (scenario._multifd_compression not in MULTIFD_CMP_ALGS)):
+                    raise Exception("unsupported multifd compression "
+                                    "algorithm: %s" %
+                                    scenario._multifd_compression)
+
             resp = src.cmd("migrate-set-capabilities",
                            capabilities = [
                                { "capability": "multifd",
@@ -206,6 +214,12 @@ def _migrate(self, hardware, scenario, src,
             resp = dst.cmd("migrate-set-parameters",
                            multifd_channels=scenario._multifd_channels)
 
+            if scenario._multifd_compression:
+                resp = src.cmd("migrate-set-parameters",
+                    multifd_compression=scenario._multifd_compression)
+                resp = dst.cmd("migrate-set-parameters",
+                    multifd_compression=scenario._multifd_compression)
+
         if scenario._dirty_limit:
             if not hardware._dirty_ring_size:
                 raise Exception("dirty ring size must be configured when "
diff --git a/scripts/migration/guestperf/scenario.py b/scripts/migration/guestperf/scenario.py
index 154c4f5d5f..4be7fafebf 100644
--- a/scripts/migration/guestperf/scenario.py
+++ b/scripts/migration/guestperf/scenario.py
@@ -30,7 +30,7 @@ def __init__(self, name,
                  auto_converge=False, auto_converge_step=10,
                  compression_mt=False, compression_mt_threads=1,
                  compression_xbzrle=False, compression_xbzrle_cache=10,
-                 multifd=False, multifd_channels=2,
+                 multifd=False, multifd_channels=2, multifd_compression="",
                  dirty_limit=False, x_vcpu_dirty_limit_period=500,
                  vcpu_dirty_limit=1):
 
@@ -61,6 +61,7 @@ def __init__(self, name,
 
         self._multifd = multifd
         self._multifd_channels = multifd_channels
+        self._multifd_compression = multifd_compression
 
         self._dirty_limit = dirty_limit
         self._x_vcpu_dirty_limit_period = x_vcpu_dirty_limit_period
@@ -85,6 +86,7 @@ def serialize(self):
             "compression_xbzrle_cache": self._compression_xbzrle_cache,
             "multifd": self._multifd,
             "multifd_channels": self._multifd_channels,
+            "multifd_compression": self._multifd_compression,
             "dirty_limit": self._dirty_limit,
             "x_vcpu_dirty_limit_period": self._x_vcpu_dirty_limit_period,
             "vcpu_dirty_limit": self._vcpu_dirty_limit,
@@ -109,4 +111,5 @@ def deserialize(cls, data):
             data["compression_xbzrle"],
             data["compression_xbzrle_cache"],
             data["multifd"],
-            data["multifd_channels"])
+            data["multifd_channels"],
+            data["multifd_compression"])
diff --git a/scripts/migration/guestperf/shell.py b/scripts/migration/guestperf/shell.py
index c85d89efec..1452eb8a33 100644
--- a/scripts/migration/guestperf/shell.py
+++ b/scripts/migration/guestperf/shell.py
@@ -130,6 +130,8 @@ def __init__(self):
                             action="store_true")
         parser.add_argument("--multifd-channels", dest="multifd_channels",
                             default=2, type=int)
+        parser.add_argument("--multifd-compression", dest="multifd_compression",
+                            default="")
 
         parser.add_argument("--dirty-limit", dest="dirty_limit", default=False,
                             action="store_true")
@@ -166,6 +168,7 @@ def get_scenario(self, args):
 
                         multifd=args.multifd,
                         multifd_channels=args.multifd_channels,
+                        multifd_compression=args.multifd_compression,
 
                         dirty_limit=args.dirty_limit,
                         x_vcpu_dirty_limit_period=\
-- 
2.27.0

Re: [PATCH v3 1/5] tests/migration: Move the guestperf tool to scripts directory

[Prasad Pandit]

On Tue, 22 Oct 2024 at 07:46, <yong.huang@smartx.com> wrote:
> Guestperf was designed to test the performance of migration,
> with a loose connection to the fundamental test cases of QEMU.
>
> To improve the repository's structure, move it to the scripts
> directory.
>
> Add myself as a maintainer for the guestperf so that I can
> help to fix bugs.
...
>  MAINTAINERS                                          | 5 +++++
>  {tests => scripts}/migration/guestperf-batch.py      | 0
>  {tests => scripts}/migration/guestperf-plot.py       | 0
>  {tests => scripts}/migration/guestperf.py            | 0
>  {tests => scripts}/migration/guestperf/__init__.py   | 0
>  {tests => scripts}/migration/guestperf/comparison.py | 0
>  {tests => scripts}/migration/guestperf/engine.py     | 0
>  {tests => scripts}/migration/guestperf/hardware.py   | 0
>  {tests => scripts}/migration/guestperf/plot.py       | 0
>  {tests => scripts}/migration/guestperf/progress.py   | 0
>  {tests => scripts}/migration/guestperf/report.py     | 0
>  {tests => scripts}/migration/guestperf/scenario.py   | 0
>  {tests => scripts}/migration/guestperf/shell.py      | 0
>  {tests => scripts}/migration/guestperf/timings.py    | 0
>  14 files changed, 5 insertions(+)
>  rename {tests => scripts}/migration/guestperf-batch.py (100%)
>  rename {tests => scripts}/migration/guestperf-plot.py (100%)
>  rename {tests => scripts}/migration/guestperf.py (100%)
>  rename {tests => scripts}/migration/guestperf/__init__.py (100%)
>  rename {tests => scripts}/migration/guestperf/comparison.py (100%)
>  rename {tests => scripts}/migration/guestperf/engine.py (100%)
>  rename {tests => scripts}/migration/guestperf/hardware.py (100%)
>  rename {tests => scripts}/migration/guestperf/plot.py (100%)
>  rename {tests => scripts}/migration/guestperf/progress.py (100%)
>  rename {tests => scripts}/migration/guestperf/report.py (100%)
>  rename {tests => scripts}/migration/guestperf/scenario.py (100%)
>  rename {tests => scripts}/migration/guestperf/shell.py (100%)
>  rename {tests => scripts}/migration/guestperf/timings.py (100%)
>

Looks okay.
Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>

Thank you.
---
  - Prasad

Re: [PATCH v3 0/5] Guestperf: miscellaneous refinement and enrichment

[Fabiano Rosas]

yong.huang@smartx.com writes:

> From: Hyman Huang <yong.huang@smartx.com>
>
> v3:
> 1. Remove the two redundant assignments in [PATCH v2 2/5] suggested by Daniel 
>
> Please review, thanks
> Yong
>
> v2:
> 1. Update the MAINTAINERS section suggested by Fabiano Rosas 
> 2. Ensure the dependencies when build the initrd-stress.img suggested by Daniel
> 3. Fix some bugs
>
> v1:
> The previous patchset:
> https://lore.kernel.org/qemu-devel/cover.1722957352.git.yong.huang@smartx.com/
> does not made the necessary changes and tests for the upstream version.
>
> This patchset works for that:
> 1. Move the guestperf to scripts directory suggested by Fabiano Rosas
> 2. Make initrd-stress.img built by default suggested by Fabiano Rosas
> 3. Make the necessary changes to adapt the latest multifd behavior
> 4. A nitpick for multifd migration
> 5. Support multifd compression option
>
> Hyman Huang (5):
>   tests/migration: Move the guestperf tool to scripts directory
>   tests/migration: Make initrd-stress.img built by default
>   guestperf: Support deferred migration for multifd
>   guestperf: Nitpick the inconsistent parameters
>   guestperf: Introduce multifd compression option
>
>  MAINTAINERS                                   |  5 +++
>  .../migration/guestperf-batch.py              |  0
>  .../migration/guestperf-plot.py               |  0
>  {tests => scripts}/migration/guestperf.py     |  0
>  .../migration/guestperf/__init__.py           |  0
>  .../migration/guestperf/comparison.py         | 15 ++++++++-
>  .../migration/guestperf/engine.py             | 33 ++++++++++++++++---
>  .../migration/guestperf/hardware.py           |  0
>  .../migration/guestperf/plot.py               |  0
>  .../migration/guestperf/progress.py           |  0
>  .../migration/guestperf/report.py             |  0
>  .../migration/guestperf/scenario.py           |  7 ++--
>  .../migration/guestperf/shell.py              |  3 ++
>  .../migration/guestperf/timings.py            |  0
>  tests/migration/meson.build                   | 30 +++++++++--------
>  15 files changed, 73 insertions(+), 20 deletions(-)
>  rename {tests => scripts}/migration/guestperf-batch.py (100%)
>  rename {tests => scripts}/migration/guestperf-plot.py (100%)
>  rename {tests => scripts}/migration/guestperf.py (100%)
>  rename {tests => scripts}/migration/guestperf/__init__.py (100%)
>  rename {tests => scripts}/migration/guestperf/comparison.py (89%)
>  rename {tests => scripts}/migration/guestperf/engine.py (93%)
>  rename {tests => scripts}/migration/guestperf/hardware.py (100%)
>  rename {tests => scripts}/migration/guestperf/plot.py (100%)
>  rename {tests => scripts}/migration/guestperf/progress.py (100%)
>  rename {tests => scripts}/migration/guestperf/report.py (100%)
>  rename {tests => scripts}/migration/guestperf/scenario.py (93%)
>  rename {tests => scripts}/migration/guestperf/shell.py (98%)
>  rename {tests => scripts}/migration/guestperf/timings.py (100%)

Reviewed-by: Fabiano Rosas <farosas@suse.de>


Thinking out loud a little bit, it's still somewhat obscure from which
directory this script should be called. None of these invocations work:

$ ./scripts/migration/guestperf.py

$ ./build/scripts/migration/guestperf.py

$ cd scripts/migration
$ ./guestperf.py

$ cd build/scripts/migration
$ ./guestperf.py

Failed to open file “tests/migration/initrd-stress.img”: open() failed:
No such file or directory

This is the only one that works:
$ cd build
$ ./scripts/migration/guestperf.py

Maybe we could improve that somehow in the future.

Re: [PATCH v3 0/5] Guestperf: miscellaneous refinement and enrichment

[Yong Huang]

[-- Attachment #1: Type: text/plain, Size: 4036 bytes --]

On Wed, Oct 23, 2024 at 11:06 PM Fabiano Rosas <farosas@suse.de> wrote:

> yong.huang@smartx.com writes:
>
> > From: Hyman Huang <yong.huang@smartx.com>
> >
> > v3:
> > 1. Remove the two redundant assignments in [PATCH v2 2/5] suggested by
> Daniel
> >
> > Please review, thanks
> > Yong
> >
> > v2:
> > 1. Update the MAINTAINERS section suggested by Fabiano Rosas
> > 2. Ensure the dependencies when build the initrd-stress.img suggested by
> Daniel
> > 3. Fix some bugs
> >
> > v1:
> > The previous patchset:
> >
> https://lore.kernel.org/qemu-devel/cover.1722957352.git.yong.huang@smartx.com/
> > does not made the necessary changes and tests for the upstream version.
> >
> > This patchset works for that:
> > 1. Move the guestperf to scripts directory suggested by Fabiano Rosas
> > 2. Make initrd-stress.img built by default suggested by Fabiano Rosas
> > 3. Make the necessary changes to adapt the latest multifd behavior
> > 4. A nitpick for multifd migration
> > 5. Support multifd compression option
> >
> > Hyman Huang (5):
> >   tests/migration: Move the guestperf tool to scripts directory
> >   tests/migration: Make initrd-stress.img built by default
> >   guestperf: Support deferred migration for multifd
> >   guestperf: Nitpick the inconsistent parameters
> >   guestperf: Introduce multifd compression option
> >
> >  MAINTAINERS                                   |  5 +++
> >  .../migration/guestperf-batch.py              |  0
> >  .../migration/guestperf-plot.py               |  0
> >  {tests => scripts}/migration/guestperf.py     |  0
> >  .../migration/guestperf/__init__.py           |  0
> >  .../migration/guestperf/comparison.py         | 15 ++++++++-
> >  .../migration/guestperf/engine.py             | 33 ++++++++++++++++---
> >  .../migration/guestperf/hardware.py           |  0
> >  .../migration/guestperf/plot.py               |  0
> >  .../migration/guestperf/progress.py           |  0
> >  .../migration/guestperf/report.py             |  0
> >  .../migration/guestperf/scenario.py           |  7 ++--
> >  .../migration/guestperf/shell.py              |  3 ++
> >  .../migration/guestperf/timings.py            |  0
> >  tests/migration/meson.build                   | 30 +++++++++--------
> >  15 files changed, 73 insertions(+), 20 deletions(-)
> >  rename {tests => scripts}/migration/guestperf-batch.py (100%)
> >  rename {tests => scripts}/migration/guestperf-plot.py (100%)
> >  rename {tests => scripts}/migration/guestperf.py (100%)
> >  rename {tests => scripts}/migration/guestperf/__init__.py (100%)
> >  rename {tests => scripts}/migration/guestperf/comparison.py (89%)
> >  rename {tests => scripts}/migration/guestperf/engine.py (93%)
> >  rename {tests => scripts}/migration/guestperf/hardware.py (100%)
> >  rename {tests => scripts}/migration/guestperf/plot.py (100%)
> >  rename {tests => scripts}/migration/guestperf/progress.py (100%)
> >  rename {tests => scripts}/migration/guestperf/report.py (100%)
> >  rename {tests => scripts}/migration/guestperf/scenario.py (93%)
> >  rename {tests => scripts}/migration/guestperf/shell.py (98%)
> >  rename {tests => scripts}/migration/guestperf/timings.py (100%)
>
> Reviewed-by: Fabiano Rosas <farosas@suse.de>
>
>
> Thinking out loud a little bit, it's still somewhat obscure from which

directory this script should be called. None of these invocations work:
>

Indeed, we could document how to build and use the guestperf,
refining it to be more convenient to use.


> $ ./scripts/migration/guestperf.py
>
> $ ./build/scripts/migration/guestperf.py
>
> $ cd scripts/migration
> $ ./guestperf.py
>
> $ cd build/scripts/migration
> $ ./guestperf.py
>
> Failed to open file “tests/migration/initrd-stress.img”: open() failed:
> No such file or directory
>
> This is the only one that works:
> $ cd build
> $ ./scripts/migration/guestperf.py
>
> Maybe we could improve that somehow in the future.
>
>

-- 
Best regards

[-- Attachment #2: Type: text/html, Size: 6095 bytes --]

Re: [PATCH v3 0/5] Guestperf: miscellaneous refinement and enrichment

[Peter Xu]

On Thu, Oct 24, 2024 at 10:06:51AM +0800, Yong Huang wrote:
> Indeed, we could document how to build and use the guestperf,
> refining it to be more convenient to use.

If to do so, please feel free to add a "test" entry in devel/migration too
no matter to put the doc there, or just add a reference to where the doc
locates.

Thanks,

-- 
Peter Xu

Re: [PATCH v3 0/5] Guestperf: miscellaneous refinement and enrichment

[Yong Huang]

[-- Attachment #1: Type: text/plain, Size: 4016 bytes --]

On Wed, Oct 23, 2024 at 11:06 PM Fabiano Rosas <farosas@suse.de> wrote:

> yong.huang@smartx.com writes:
>
> > From: Hyman Huang <yong.huang@smartx.com>
> >
> > v3:
> > 1. Remove the two redundant assignments in [PATCH v2 2/5] suggested by
> Daniel
> >
> > Please review, thanks
> > Yong
> >
> > v2:
> > 1. Update the MAINTAINERS section suggested by Fabiano Rosas
> > 2. Ensure the dependencies when build the initrd-stress.img suggested by
> Daniel
> > 3. Fix some bugs
> >
> > v1:
> > The previous patchset:
> >
> https://lore.kernel.org/qemu-devel/cover.1722957352.git.yong.huang@smartx.com/
> > does not made the necessary changes and tests for the upstream version.
> >
> > This patchset works for that:
> > 1. Move the guestperf to scripts directory suggested by Fabiano Rosas
> > 2. Make initrd-stress.img built by default suggested by Fabiano Rosas
> > 3. Make the necessary changes to adapt the latest multifd behavior
> > 4. A nitpick for multifd migration
> > 5. Support multifd compression option
> >
> > Hyman Huang (5):
> >   tests/migration: Move the guestperf tool to scripts directory
> >   tests/migration: Make initrd-stress.img built by default
> >   guestperf: Support deferred migration for multifd
> >   guestperf: Nitpick the inconsistent parameters
> >   guestperf: Introduce multifd compression option
> >
> >  MAINTAINERS                                   |  5 +++
> >  .../migration/guestperf-batch.py              |  0
> >  .../migration/guestperf-plot.py               |  0
> >  {tests => scripts}/migration/guestperf.py     |  0
> >  .../migration/guestperf/__init__.py           |  0
> >  .../migration/guestperf/comparison.py         | 15 ++++++++-
> >  .../migration/guestperf/engine.py             | 33 ++++++++++++++++---
> >  .../migration/guestperf/hardware.py           |  0
> >  .../migration/guestperf/plot.py               |  0
> >  .../migration/guestperf/progress.py           |  0
> >  .../migration/guestperf/report.py             |  0
> >  .../migration/guestperf/scenario.py           |  7 ++--
> >  .../migration/guestperf/shell.py              |  3 ++
> >  .../migration/guestperf/timings.py            |  0
> >  tests/migration/meson.build                   | 30 +++++++++--------
> >  15 files changed, 73 insertions(+), 20 deletions(-)
> >  rename {tests => scripts}/migration/guestperf-batch.py (100%)
> >  rename {tests => scripts}/migration/guestperf-plot.py (100%)
> >  rename {tests => scripts}/migration/guestperf.py (100%)
> >  rename {tests => scripts}/migration/guestperf/__init__.py (100%)
> >  rename {tests => scripts}/migration/guestperf/comparison.py (89%)
> >  rename {tests => scripts}/migration/guestperf/engine.py (93%)
> >  rename {tests => scripts}/migration/guestperf/hardware.py (100%)
> >  rename {tests => scripts}/migration/guestperf/plot.py (100%)
> >  rename {tests => scripts}/migration/guestperf/progress.py (100%)
> >  rename {tests => scripts}/migration/guestperf/report.py (100%)
> >  rename {tests => scripts}/migration/guestperf/scenario.py (93%)
> >  rename {tests => scripts}/migration/guestperf/shell.py (98%)
> >  rename {tests => scripts}/migration/guestperf/timings.py (100%)
>
> Reviewed-by: Fabiano Rosas <farosas@suse.de>
>
>
> Thinking out loud a little bit, it's still somewhat obscure from which
> directory this script should be called. None of these invocations work:
>
> $ ./scripts/migration/guestperf.py
>
> $ ./build/scripts/migration/guestperf.py
>
> $ cd scripts/migration
> $ ./guestperf.py
>
> $ cd build/scripts/migration
> $ ./guestperf.py
>
> Failed to open file “tests/migration/initrd-stress.img”: open() failed:
> No such file or directory
>
> This is the only one that works:
> $ cd build
> $ ./scripts/migration/guestperf.py
>
> Maybe we could improve that somehow in the future.
>
>
This patchset could be queued now,
Or after refining the documents mentioned above later?

-- 
Best regards

[-- Attachment #2: Type: text/html, Size: 5733 bytes --]

Re: [PATCH v3 0/5] Guestperf: miscellaneous refinement and enrichment

[Fabiano Rosas]

Yong Huang <yong.huang@smartx.com> writes:

> On Wed, Oct 23, 2024 at 11:06 PM Fabiano Rosas <farosas@suse.de> wrote:
>
>> yong.huang@smartx.com writes:
>>
>> > From: Hyman Huang <yong.huang@smartx.com>
>> >
>> > v3:
>> > 1. Remove the two redundant assignments in [PATCH v2 2/5] suggested by
>> Daniel
>> >
>> > Please review, thanks
>> > Yong
>> >
>> > v2:
>> > 1. Update the MAINTAINERS section suggested by Fabiano Rosas
>> > 2. Ensure the dependencies when build the initrd-stress.img suggested by
>> Daniel
>> > 3. Fix some bugs
>> >
>> > v1:
>> > The previous patchset:
>> >
>> https://lore.kernel.org/qemu-devel/cover.1722957352.git.yong.huang@smartx.com/
>> > does not made the necessary changes and tests for the upstream version.
>> >
>> > This patchset works for that:
>> > 1. Move the guestperf to scripts directory suggested by Fabiano Rosas
>> > 2. Make initrd-stress.img built by default suggested by Fabiano Rosas
>> > 3. Make the necessary changes to adapt the latest multifd behavior
>> > 4. A nitpick for multifd migration
>> > 5. Support multifd compression option
>> >
>> > Hyman Huang (5):
>> >   tests/migration: Move the guestperf tool to scripts directory
>> >   tests/migration: Make initrd-stress.img built by default
>> >   guestperf: Support deferred migration for multifd
>> >   guestperf: Nitpick the inconsistent parameters
>> >   guestperf: Introduce multifd compression option
>> >
>> >  MAINTAINERS                                   |  5 +++
>> >  .../migration/guestperf-batch.py              |  0
>> >  .../migration/guestperf-plot.py               |  0
>> >  {tests => scripts}/migration/guestperf.py     |  0
>> >  .../migration/guestperf/__init__.py           |  0
>> >  .../migration/guestperf/comparison.py         | 15 ++++++++-
>> >  .../migration/guestperf/engine.py             | 33 ++++++++++++++++---
>> >  .../migration/guestperf/hardware.py           |  0
>> >  .../migration/guestperf/plot.py               |  0
>> >  .../migration/guestperf/progress.py           |  0
>> >  .../migration/guestperf/report.py             |  0
>> >  .../migration/guestperf/scenario.py           |  7 ++--
>> >  .../migration/guestperf/shell.py              |  3 ++
>> >  .../migration/guestperf/timings.py            |  0
>> >  tests/migration/meson.build                   | 30 +++++++++--------
>> >  15 files changed, 73 insertions(+), 20 deletions(-)
>> >  rename {tests => scripts}/migration/guestperf-batch.py (100%)
>> >  rename {tests => scripts}/migration/guestperf-plot.py (100%)
>> >  rename {tests => scripts}/migration/guestperf.py (100%)
>> >  rename {tests => scripts}/migration/guestperf/__init__.py (100%)
>> >  rename {tests => scripts}/migration/guestperf/comparison.py (89%)
>> >  rename {tests => scripts}/migration/guestperf/engine.py (93%)
>> >  rename {tests => scripts}/migration/guestperf/hardware.py (100%)
>> >  rename {tests => scripts}/migration/guestperf/plot.py (100%)
>> >  rename {tests => scripts}/migration/guestperf/progress.py (100%)
>> >  rename {tests => scripts}/migration/guestperf/report.py (100%)
>> >  rename {tests => scripts}/migration/guestperf/scenario.py (93%)
>> >  rename {tests => scripts}/migration/guestperf/shell.py (98%)
>> >  rename {tests => scripts}/migration/guestperf/timings.py (100%)
>>
>> Reviewed-by: Fabiano Rosas <farosas@suse.de>
>>
>>
>> Thinking out loud a little bit, it's still somewhat obscure from which
>> directory this script should be called. None of these invocations work:
>>
>> $ ./scripts/migration/guestperf.py
>>
>> $ ./build/scripts/migration/guestperf.py
>>
>> $ cd scripts/migration
>> $ ./guestperf.py
>>
>> $ cd build/scripts/migration
>> $ ./guestperf.py
>>
>> Failed to open file “tests/migration/initrd-stress.img”: open() failed:
>> No such file or directory
>>
>> This is the only one that works:
>> $ cd build
>> $ ./scripts/migration/guestperf.py
>>
>> Maybe we could improve that somehow in the future.
>>
>>
> This patchset could be queued now,
> Or after refining the documents mentioned above later?

I think it's ok. Let me just rebase this on top of the other series that
touches the tests/migration/ directory. I'll repost soon.

Re: [PATCH v3 0/5] Guestperf: miscellaneous refinement and enrichment

[Fabiano Rosas]

Fabiano Rosas <farosas@suse.de> writes:

> Yong Huang <yong.huang@smartx.com> writes:
>
>> On Wed, Oct 23, 2024 at 11:06 PM Fabiano Rosas <farosas@suse.de> wrote:
>>
>>> yong.huang@smartx.com writes:
>>>
>>> > From: Hyman Huang <yong.huang@smartx.com>
>>> >
>>> > v3:
>>> > 1. Remove the two redundant assignments in [PATCH v2 2/5] suggested by
>>> Daniel
>>> >
>>> > Please review, thanks
>>> > Yong
>>> >
>>> > v2:
>>> > 1. Update the MAINTAINERS section suggested by Fabiano Rosas
>>> > 2. Ensure the dependencies when build the initrd-stress.img suggested by
>>> Daniel
>>> > 3. Fix some bugs
>>> >
>>> > v1:
>>> > The previous patchset:
>>> >
>>> https://lore.kernel.org/qemu-devel/cover.1722957352.git.yong.huang@smartx.com/
>>> > does not made the necessary changes and tests for the upstream version.
>>> >
>>> > This patchset works for that:
>>> > 1. Move the guestperf to scripts directory suggested by Fabiano Rosas
>>> > 2. Make initrd-stress.img built by default suggested by Fabiano Rosas
>>> > 3. Make the necessary changes to adapt the latest multifd behavior
>>> > 4. A nitpick for multifd migration
>>> > 5. Support multifd compression option
>>> >
>>> > Hyman Huang (5):
>>> >   tests/migration: Move the guestperf tool to scripts directory
>>> >   tests/migration: Make initrd-stress.img built by default
>>> >   guestperf: Support deferred migration for multifd
>>> >   guestperf: Nitpick the inconsistent parameters
>>> >   guestperf: Introduce multifd compression option
>>> >
>>> >  MAINTAINERS                                   |  5 +++
>>> >  .../migration/guestperf-batch.py              |  0
>>> >  .../migration/guestperf-plot.py               |  0
>>> >  {tests => scripts}/migration/guestperf.py     |  0
>>> >  .../migration/guestperf/__init__.py           |  0
>>> >  .../migration/guestperf/comparison.py         | 15 ++++++++-
>>> >  .../migration/guestperf/engine.py             | 33 ++++++++++++++++---
>>> >  .../migration/guestperf/hardware.py           |  0
>>> >  .../migration/guestperf/plot.py               |  0
>>> >  .../migration/guestperf/progress.py           |  0
>>> >  .../migration/guestperf/report.py             |  0
>>> >  .../migration/guestperf/scenario.py           |  7 ++--
>>> >  .../migration/guestperf/shell.py              |  3 ++
>>> >  .../migration/guestperf/timings.py            |  0
>>> >  tests/migration/meson.build                   | 30 +++++++++--------
>>> >  15 files changed, 73 insertions(+), 20 deletions(-)
>>> >  rename {tests => scripts}/migration/guestperf-batch.py (100%)
>>> >  rename {tests => scripts}/migration/guestperf-plot.py (100%)
>>> >  rename {tests => scripts}/migration/guestperf.py (100%)
>>> >  rename {tests => scripts}/migration/guestperf/__init__.py (100%)
>>> >  rename {tests => scripts}/migration/guestperf/comparison.py (89%)
>>> >  rename {tests => scripts}/migration/guestperf/engine.py (93%)
>>> >  rename {tests => scripts}/migration/guestperf/hardware.py (100%)
>>> >  rename {tests => scripts}/migration/guestperf/plot.py (100%)
>>> >  rename {tests => scripts}/migration/guestperf/progress.py (100%)
>>> >  rename {tests => scripts}/migration/guestperf/report.py (100%)
>>> >  rename {tests => scripts}/migration/guestperf/scenario.py (93%)
>>> >  rename {tests => scripts}/migration/guestperf/shell.py (98%)
>>> >  rename {tests => scripts}/migration/guestperf/timings.py (100%)
>>>
>>> Reviewed-by: Fabiano Rosas <farosas@suse.de>
>>>
>>>
>>> Thinking out loud a little bit, it's still somewhat obscure from which
>>> directory this script should be called. None of these invocations work:
>>>
>>> $ ./scripts/migration/guestperf.py
>>>
>>> $ ./build/scripts/migration/guestperf.py
>>>
>>> $ cd scripts/migration
>>> $ ./guestperf.py
>>>
>>> $ cd build/scripts/migration
>>> $ ./guestperf.py
>>>
>>> Failed to open file “tests/migration/initrd-stress.img”: open() failed:
>>> No such file or directory
>>>
>>> This is the only one that works:
>>> $ cd build
>>> $ ./scripts/migration/guestperf.py
>>>
>>> Maybe we could improve that somehow in the future.
>>>
>>>
>> This patchset could be queued now,
>> Or after refining the documents mentioned above later?
>
> I think it's ok. Let me just rebase this on top of the other series that
> touches the tests/migration/ directory. I'll repost soon.

Well, not so fast, the CI doesn't like the addition of stress.c:

https://gitlab.com/farosas/qemu/-/jobs/8504697373
clang: error: argument unused during compilation: '-pie'

https://gitlab.com/farosas/qemu/-/jobs/8504697378
undefined reference to `_DYNAMIC'

Re: [PATCH v3 0/5] Guestperf: miscellaneous refinement and enrichment

[Yong Huang]

[-- Attachment #1: Type: text/plain, Size: 5083 bytes --]

On Fri, Nov 29, 2024 at 9:44 PM Fabiano Rosas <farosas@suse.de> wrote:

> Fabiano Rosas <farosas@suse.de> writes:
>
> > Yong Huang <yong.huang@smartx.com> writes:
> >
> >> On Wed, Oct 23, 2024 at 11:06 PM Fabiano Rosas <farosas@suse.de> wrote:
> >>
> >>> yong.huang@smartx.com writes:
> >>>
> >>> > From: Hyman Huang <yong.huang@smartx.com>
> >>> >
> >>> > v3:
> >>> > 1. Remove the two redundant assignments in [PATCH v2 2/5] suggested
> by
> >>> Daniel
> >>> >
> >>> > Please review, thanks
> >>> > Yong
> >>> >
> >>> > v2:
> >>> > 1. Update the MAINTAINERS section suggested by Fabiano Rosas
> >>> > 2. Ensure the dependencies when build the initrd-stress.img
> suggested by
> >>> Daniel
> >>> > 3. Fix some bugs
> >>> >
> >>> > v1:
> >>> > The previous patchset:
> >>> >
> >>>
> https://lore.kernel.org/qemu-devel/cover.1722957352.git.yong.huang@smartx.com/
> >>> > does not made the necessary changes and tests for the upstream
> version.
> >>> >
> >>> > This patchset works for that:
> >>> > 1. Move the guestperf to scripts directory suggested by Fabiano Rosas
> >>> > 2. Make initrd-stress.img built by default suggested by Fabiano Rosas
> >>> > 3. Make the necessary changes to adapt the latest multifd behavior
> >>> > 4. A nitpick for multifd migration
> >>> > 5. Support multifd compression option
> >>> >
> >>> > Hyman Huang (5):
> >>> >   tests/migration: Move the guestperf tool to scripts directory
> >>> >   tests/migration: Make initrd-stress.img built by default
> >>> >   guestperf: Support deferred migration for multifd
> >>> >   guestperf: Nitpick the inconsistent parameters
> >>> >   guestperf: Introduce multifd compression option
> >>> >
> >>> >  MAINTAINERS                                   |  5 +++
> >>> >  .../migration/guestperf-batch.py              |  0
> >>> >  .../migration/guestperf-plot.py               |  0
> >>> >  {tests => scripts}/migration/guestperf.py     |  0
> >>> >  .../migration/guestperf/__init__.py           |  0
> >>> >  .../migration/guestperf/comparison.py         | 15 ++++++++-
> >>> >  .../migration/guestperf/engine.py             | 33
> ++++++++++++++++---
> >>> >  .../migration/guestperf/hardware.py           |  0
> >>> >  .../migration/guestperf/plot.py               |  0
> >>> >  .../migration/guestperf/progress.py           |  0
> >>> >  .../migration/guestperf/report.py             |  0
> >>> >  .../migration/guestperf/scenario.py           |  7 ++--
> >>> >  .../migration/guestperf/shell.py              |  3 ++
> >>> >  .../migration/guestperf/timings.py            |  0
> >>> >  tests/migration/meson.build                   | 30 +++++++++--------
> >>> >  15 files changed, 73 insertions(+), 20 deletions(-)
> >>> >  rename {tests => scripts}/migration/guestperf-batch.py (100%)
> >>> >  rename {tests => scripts}/migration/guestperf-plot.py (100%)
> >>> >  rename {tests => scripts}/migration/guestperf.py (100%)
> >>> >  rename {tests => scripts}/migration/guestperf/__init__.py (100%)
> >>> >  rename {tests => scripts}/migration/guestperf/comparison.py (89%)
> >>> >  rename {tests => scripts}/migration/guestperf/engine.py (93%)
> >>> >  rename {tests => scripts}/migration/guestperf/hardware.py (100%)
> >>> >  rename {tests => scripts}/migration/guestperf/plot.py (100%)
> >>> >  rename {tests => scripts}/migration/guestperf/progress.py (100%)
> >>> >  rename {tests => scripts}/migration/guestperf/report.py (100%)
> >>> >  rename {tests => scripts}/migration/guestperf/scenario.py (93%)
> >>> >  rename {tests => scripts}/migration/guestperf/shell.py (98%)
> >>> >  rename {tests => scripts}/migration/guestperf/timings.py (100%)
> >>>
> >>> Reviewed-by: Fabiano Rosas <farosas@suse.de>
> >>>
> >>>
> >>> Thinking out loud a little bit, it's still somewhat obscure from which
> >>> directory this script should be called. None of these invocations work:
> >>>
> >>> $ ./scripts/migration/guestperf.py
> >>>
> >>> $ ./build/scripts/migration/guestperf.py
> >>>
> >>> $ cd scripts/migration
> >>> $ ./guestperf.py
> >>>
> >>> $ cd build/scripts/migration
> >>> $ ./guestperf.py
> >>>
> >>> Failed to open file “tests/migration/initrd-stress.img”: open() failed:
> >>> No such file or directory
> >>>
> >>> This is the only one that works:
> >>> $ cd build
> >>> $ ./scripts/migration/guestperf.py
> >>>
> >>> Maybe we could improve that somehow in the future.
> >>>
> >>>
> >> This patchset could be queued now,
> >> Or after refining the documents mentioned above later?
> >
> > I think it's ok. Let me just rebase this on top of the other series that
> > touches the tests/migration/ directory. I'll repost soon.
>
> Well, not so fast, the CI doesn't like the addition of stress.c:
>
> https://gitlab.com/farosas/qemu/-/jobs/8504697373
> clang: error: argument unused during compilation: '-pie'


> https://gitlab.com/farosas/qemu/-/jobs/8504697378
> undefined reference to `_DYNAMIC'
>

OK, I'll fix that once I have time.

Thanks,
Yong

-- 
Best regards

[-- Attachment #2: Type: text/html, Size: 8384 bytes --]

Re: [PATCH v3 0/5] Guestperf: miscellaneous refinement and enrichment

[Peter Xu]

On Sat, Nov 30, 2024 at 10:01:43PM +0800, Yong Huang wrote:
> On Fri, Nov 29, 2024 at 9:44 PM Fabiano Rosas <farosas@suse.de> wrote:
> 
> > Fabiano Rosas <farosas@suse.de> writes:
> >
> > > Yong Huang <yong.huang@smartx.com> writes:
> > >
> > >> On Wed, Oct 23, 2024 at 11:06 PM Fabiano Rosas <farosas@suse.de> wrote:
> > >>
> > >>> yong.huang@smartx.com writes:
> > >>>
> > >>> > From: Hyman Huang <yong.huang@smartx.com>
> > >>> >
> > >>> > v3:
> > >>> > 1. Remove the two redundant assignments in [PATCH v2 2/5] suggested
> > by
> > >>> Daniel
> > >>> >
> > >>> > Please review, thanks
> > >>> > Yong
> > >>> >
> > >>> > v2:
> > >>> > 1. Update the MAINTAINERS section suggested by Fabiano Rosas
> > >>> > 2. Ensure the dependencies when build the initrd-stress.img
> > suggested by
> > >>> Daniel
> > >>> > 3. Fix some bugs
> > >>> >
> > >>> > v1:
> > >>> > The previous patchset:
> > >>> >
> > >>>
> > https://lore.kernel.org/qemu-devel/cover.1722957352.git.yong.huang@smartx.com/
> > >>> > does not made the necessary changes and tests for the upstream
> > version.
> > >>> >
> > >>> > This patchset works for that:
> > >>> > 1. Move the guestperf to scripts directory suggested by Fabiano Rosas
> > >>> > 2. Make initrd-stress.img built by default suggested by Fabiano Rosas
> > >>> > 3. Make the necessary changes to adapt the latest multifd behavior
> > >>> > 4. A nitpick for multifd migration
> > >>> > 5. Support multifd compression option
> > >>> >
> > >>> > Hyman Huang (5):
> > >>> >   tests/migration: Move the guestperf tool to scripts directory
> > >>> >   tests/migration: Make initrd-stress.img built by default
> > >>> >   guestperf: Support deferred migration for multifd
> > >>> >   guestperf: Nitpick the inconsistent parameters
> > >>> >   guestperf: Introduce multifd compression option
> > >>> >
> > >>> >  MAINTAINERS                                   |  5 +++
> > >>> >  .../migration/guestperf-batch.py              |  0
> > >>> >  .../migration/guestperf-plot.py               |  0
> > >>> >  {tests => scripts}/migration/guestperf.py     |  0
> > >>> >  .../migration/guestperf/__init__.py           |  0
> > >>> >  .../migration/guestperf/comparison.py         | 15 ++++++++-
> > >>> >  .../migration/guestperf/engine.py             | 33
> > ++++++++++++++++---
> > >>> >  .../migration/guestperf/hardware.py           |  0
> > >>> >  .../migration/guestperf/plot.py               |  0
> > >>> >  .../migration/guestperf/progress.py           |  0
> > >>> >  .../migration/guestperf/report.py             |  0
> > >>> >  .../migration/guestperf/scenario.py           |  7 ++--
> > >>> >  .../migration/guestperf/shell.py              |  3 ++
> > >>> >  .../migration/guestperf/timings.py            |  0
> > >>> >  tests/migration/meson.build                   | 30 +++++++++--------
> > >>> >  15 files changed, 73 insertions(+), 20 deletions(-)
> > >>> >  rename {tests => scripts}/migration/guestperf-batch.py (100%)
> > >>> >  rename {tests => scripts}/migration/guestperf-plot.py (100%)
> > >>> >  rename {tests => scripts}/migration/guestperf.py (100%)
> > >>> >  rename {tests => scripts}/migration/guestperf/__init__.py (100%)
> > >>> >  rename {tests => scripts}/migration/guestperf/comparison.py (89%)
> > >>> >  rename {tests => scripts}/migration/guestperf/engine.py (93%)
> > >>> >  rename {tests => scripts}/migration/guestperf/hardware.py (100%)
> > >>> >  rename {tests => scripts}/migration/guestperf/plot.py (100%)
> > >>> >  rename {tests => scripts}/migration/guestperf/progress.py (100%)
> > >>> >  rename {tests => scripts}/migration/guestperf/report.py (100%)
> > >>> >  rename {tests => scripts}/migration/guestperf/scenario.py (93%)
> > >>> >  rename {tests => scripts}/migration/guestperf/shell.py (98%)
> > >>> >  rename {tests => scripts}/migration/guestperf/timings.py (100%)
> > >>>
> > >>> Reviewed-by: Fabiano Rosas <farosas@suse.de>
> > >>>
> > >>>
> > >>> Thinking out loud a little bit, it's still somewhat obscure from which
> > >>> directory this script should be called. None of these invocations work:
> > >>>
> > >>> $ ./scripts/migration/guestperf.py
> > >>>
> > >>> $ ./build/scripts/migration/guestperf.py
> > >>>
> > >>> $ cd scripts/migration
> > >>> $ ./guestperf.py
> > >>>
> > >>> $ cd build/scripts/migration
> > >>> $ ./guestperf.py
> > >>>
> > >>> Failed to open file “tests/migration/initrd-stress.img”: open() failed:
> > >>> No such file or directory
> > >>>
> > >>> This is the only one that works:
> > >>> $ cd build
> > >>> $ ./scripts/migration/guestperf.py
> > >>>
> > >>> Maybe we could improve that somehow in the future.
> > >>>
> > >>>
> > >> This patchset could be queued now,
> > >> Or after refining the documents mentioned above later?
> > >
> > > I think it's ok. Let me just rebase this on top of the other series that
> > > touches the tests/migration/ directory. I'll repost soon.
> >
> > Well, not so fast, the CI doesn't like the addition of stress.c:
> >
> > https://gitlab.com/farosas/qemu/-/jobs/8504697373
> > clang: error: argument unused during compilation: '-pie'
> 
> 
> > https://gitlab.com/farosas/qemu/-/jobs/8504697378
> > undefined reference to `_DYNAMIC'
> >
> 
> OK, I'll fix that once I have time.

Yong,

Note that the conflict is against this patch (now in migration-next, so
will be in 10.0's pull):

https://lore.kernel.org/r/20241127182901.529-4-farosas@suse.de

When you rebase, feel free to do that on top of migration-next if it didn't
yet land master branch, could be easier for all of us:

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

For the stress.c warnings/error, I saw another one warning with gcc when
built it locally, then I found Fabiano's CI run also has it:

https://gitlab.com/farosas/qemu/-/jobs/8504697347

Which has:

[3381/3611] Linking target tests/migration/stress
/usr/bin/ld: /usr/lib64/libglib-2.0.a(gutils.c.o): in function `g_get_user_database_entry':
(.text+0xeb): warning: Using 'getpwnam_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
/usr/bin/ld: (.text+0x2be): warning: Using 'getpwuid' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
/usr/bin/ld: (.text+0x134): warning: Using 'getpwuid_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking

Feel free to look at it too if you like. Nothing should be relevant to your
changes, so I think it could be there for a while when compilers upgrade.

Thanks,

-- 
Peter Xu

Re: [PATCH v3 0/5] Guestperf: miscellaneous refinement and enrichment

[Daniel P. Berrangé]

On Mon, Dec 02, 2024 at 02:49:33PM -0500, Peter Xu wrote:
> On Sat, Nov 30, 2024 at 10:01:43PM +0800, Yong Huang wrote:
> > On Fri, Nov 29, 2024 at 9:44 PM Fabiano Rosas <farosas@suse.de> wrote:
> For the stress.c warnings/error, I saw another one warning with gcc when
> built it locally, then I found Fabiano's CI run also has it:
> 
> https://gitlab.com/farosas/qemu/-/jobs/8504697347
> 
> Which has:
> 
> [3381/3611] Linking target tests/migration/stress
> /usr/bin/ld: /usr/lib64/libglib-2.0.a(gutils.c.o): in function `g_get_user_database_entry':
> (.text+0xeb): warning: Using 'getpwnam_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linkign
> /usr/bin/ld: (.text+0x2be): warning: Using 'getpwuid' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
> /usr/bin/ld: (.text+0x134): warning: Using 'getpwuid_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
> 
> Feel free to look at it too if you like. Nothing should be relevant to your
> changes, so I think it could be there for a while when compilers upgrade.

Those messages are mostly harmless, especially in the stress scenario, and
can't practically be eliminated:

  https://gitlab.gnome.org/GNOME/glib/-/issues/2019

With 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 :|

Re: [PATCH v3 0/5] Guestperf: miscellaneous refinement and enrichment

[Fabiano Rosas]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Mon, Dec 02, 2024 at 02:49:33PM -0500, Peter Xu wrote:
>> On Sat, Nov 30, 2024 at 10:01:43PM +0800, Yong Huang wrote:
>> > On Fri, Nov 29, 2024 at 9:44 PM Fabiano Rosas <farosas@suse.de> wrote:
>> For the stress.c warnings/error, I saw another one warning with gcc when
>> built it locally, then I found Fabiano's CI run also has it:
>> 
>> https://gitlab.com/farosas/qemu/-/jobs/8504697347
>> 
>> Which has:
>> 
>> [3381/3611] Linking target tests/migration/stress
>> /usr/bin/ld: /usr/lib64/libglib-2.0.a(gutils.c.o): in function `g_get_user_database_entry':
>> (.text+0xeb): warning: Using 'getpwnam_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linkign
>> /usr/bin/ld: (.text+0x2be): warning: Using 'getpwuid' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
>> /usr/bin/ld: (.text+0x134): warning: Using 'getpwuid_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
>> 
>> Feel free to look at it too if you like. Nothing should be relevant to your
>> changes, so I think it could be there for a while when compilers upgrade.
>
> Those messages are mostly harmless, especially in the stress scenario, and
> can't practically be eliminated:
>
>   https://gitlab.gnome.org/GNOME/glib/-/issues/2019

Although harmless, we can't have them showing up during the
build. Should we go back to not building stress.c by default? Another
option (which I prefer) would be to stop using glib in stress.c, it
doesn't look like it would be much work to do that. 

Re: [PATCH v3 0/5] Guestperf: miscellaneous refinement and enrichment

[Daniel P. Berrangé]

On Tue, Dec 03, 2024 at 09:20:44AM -0300, Fabiano Rosas wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Mon, Dec 02, 2024 at 02:49:33PM -0500, Peter Xu wrote:
> >> On Sat, Nov 30, 2024 at 10:01:43PM +0800, Yong Huang wrote:
> >> > On Fri, Nov 29, 2024 at 9:44 PM Fabiano Rosas <farosas@suse.de> wrote:
> >> For the stress.c warnings/error, I saw another one warning with gcc when
> >> built it locally, then I found Fabiano's CI run also has it:
> >> 
> >> https://gitlab.com/farosas/qemu/-/jobs/8504697347
> >> 
> >> Which has:
> >> 
> >> [3381/3611] Linking target tests/migration/stress
> >> /usr/bin/ld: /usr/lib64/libglib-2.0.a(gutils.c.o): in function `g_get_user_database_entry':
> >> (.text+0xeb): warning: Using 'getpwnam_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linkign
> >> /usr/bin/ld: (.text+0x2be): warning: Using 'getpwuid' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
> >> /usr/bin/ld: (.text+0x134): warning: Using 'getpwuid_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
> >> 
> >> Feel free to look at it too if you like. Nothing should be relevant to your
> >> changes, so I think it could be there for a while when compilers upgrade.
> >
> > Those messages are mostly harmless, especially in the stress scenario, and
> > can't practically be eliminated:
> >
> >   https://gitlab.gnome.org/GNOME/glib/-/issues/2019
> 
> Although harmless, we can't have them showing up during the
> build. Should we go back to not building stress.c by default? Another
> option (which I prefer) would be to stop using glib in stress.c, it
> doesn't look like it would be much work to do that. 

FYI they already show up in QEMU builds if you are building qemu-user as
static binaries, which is what you want for binfmt usage.  The main
diff is that in this case they'll show up by default for anyone who has
glib-static available.  Is that a big problem ?


With 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 :|

Re: [PATCH v3 0/5] Guestperf: miscellaneous refinement and enrichment

[Fabiano Rosas]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Dec 03, 2024 at 09:20:44AM -0300, Fabiano Rosas wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>> 
>> > On Mon, Dec 02, 2024 at 02:49:33PM -0500, Peter Xu wrote:
>> >> On Sat, Nov 30, 2024 at 10:01:43PM +0800, Yong Huang wrote:
>> >> > On Fri, Nov 29, 2024 at 9:44 PM Fabiano Rosas <farosas@suse.de> wrote:
>> >> For the stress.c warnings/error, I saw another one warning with gcc when
>> >> built it locally, then I found Fabiano's CI run also has it:
>> >> 
>> >> https://gitlab.com/farosas/qemu/-/jobs/8504697347
>> >> 
>> >> Which has:
>> >> 
>> >> [3381/3611] Linking target tests/migration/stress
>> >> /usr/bin/ld: /usr/lib64/libglib-2.0.a(gutils.c.o): in function `g_get_user_database_entry':
>> >> (.text+0xeb): warning: Using 'getpwnam_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linkign
>> >> /usr/bin/ld: (.text+0x2be): warning: Using 'getpwuid' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
>> >> /usr/bin/ld: (.text+0x134): warning: Using 'getpwuid_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
>> >> 
>> >> Feel free to look at it too if you like. Nothing should be relevant to your
>> >> changes, so I think it could be there for a while when compilers upgrade.
>> >
>> > Those messages are mostly harmless, especially in the stress scenario, and
>> > can't practically be eliminated:
>> >
>> >   https://gitlab.gnome.org/GNOME/glib/-/issues/2019
>> 
>> Although harmless, we can't have them showing up during the
>> build. Should we go back to not building stress.c by default? Another
>> option (which I prefer) would be to stop using glib in stress.c, it
>> doesn't look like it would be much work to do that. 
>
> FYI they already show up in QEMU builds if you are building qemu-user as
> static binaries, which is what you want for binfmt usage.  The main
> diff is that in this case they'll show up by default for anyone who has
> glib-static available.  Is that a big problem ?

We shouldn't be adding warnings to the build like that. When building
static binaries, I'd assume the person at least knows there's a -static
in there somewhere. If you're just building the system binaries and
warnings start to show up, that's not good. Since this is just a side
script that's very infrequently used, I don't think it justifies the
extra warning.

Maybe let's see what the other build issues were, I'm guessing it will
be more work to fix them than to disable stress.c build again.

>
>
> With regards,
> Daniel

Re: [PATCH v3 0/5] Guestperf: miscellaneous refinement and enrichment

[Peter Xu]

On Tue, Dec 03, 2024 at 10:15:57AM -0300, Fabiano Rosas wrote:
> We shouldn't be adding warnings to the build like that. When building
> static binaries, I'd assume the person at least knows there's a -static
> in there somewhere. If you're just building the system binaries and
> warnings start to show up, that's not good. Since this is just a side
> script that's very infrequently used, I don't think it justifies the
> extra warning.

Yeah this could be a valid point.

The main issue is I believe 99.999999% of people building qemu will not use
stress.c and the initrd at all.  It means we could start burning some tiny
little more cpus all over the worlds for nothing.. the added warning is a
bad extra side effect of that.

So I wonder if it would make more sense to only build stress.c manually
like before, until some of the stress test would be put into either 'make
check' or CI flows.  Then we decide whether to fix the warning or not.

-- 
Peter Xu

Re: [PATCH v3 0/5] Guestperf: miscellaneous refinement and enrichment

[Yong Huang]

[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]

On Wed, Dec 4, 2024 at 1:40 AM Peter Xu <peterx@redhat.com> wrote:

> On Tue, Dec 03, 2024 at 10:15:57AM -0300, Fabiano Rosas wrote:
> > We shouldn't be adding warnings to the build like that. When building
> > static binaries, I'd assume the person at least knows there's a -static
> > in there somewhere. If you're just building the system binaries and
> > warnings start to show up, that's not good. Since this is just a side
> > script that's very infrequently used, I don't think it justifies the
> > extra warning.
>
> Yeah this could be a valid point.
>
> The main issue is I believe 99.999999% of people building qemu will not use
> stress.c and the initrd at all.  It means we could start burning some tiny
> little more cpus all over the worlds for nothing.. the added warning is a
> bad extra side effect of that.
>
> So I wonder if it would make more sense to only build stress.c manually
>

Ok, get it.


> like before, until some of the stress test would be put into either 'make
> check' or CI flows.  Then we decide whether to fix the warning or not.
>

Yes, I think that adding the essential guestperf test to "make check"
(like migration via a Unix socket) may make sense, at least from the
perspective of guestperf's usability.


> --
> Peter Xu
>
>

-- 
Best regards

[-- Attachment #2: Type: text/html, Size: 2866 bytes --]