On Tue, Sep 30, 2025 at 02:23:49PM -0400, Steven Sistare wrote:
> On 9/30/2025 1:08 PM, Peter Xu wrote:
> > On Fri, Sep 19, 2025 at 07:12:33AM -0700, Steve Sistare wrote:
> > > Add a test for the cpr-exec migration mode.
> > >
> > > Signed-off-by: Steve Sistare <[email protected]>
> >
> > Looks good, only some nitpicks or pure questions below.
> >
> > > ---
> > > tests/qtest/migration/cpr-tests.c | 120
> > > ++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 120 insertions(+)
> > >
> > > diff --git a/tests/qtest/migration/cpr-tests.c
> > > b/tests/qtest/migration/cpr-tests.c
> > > index 5e764a6..f33af76 100644
> > > --- a/tests/qtest/migration/cpr-tests.c
> > > +++ b/tests/qtest/migration/cpr-tests.c
> > > @@ -110,6 +110,125 @@ static void test_mode_transfer_defer(void)
> > > test_mode_transfer_common(true);
> > > }
> > > +static void set_cpr_exec_args(QTestState *who, MigrateCommon *args)
> > > +{
> > > + g_autofree char *qtest_from_args = NULL;
> > > + g_autofree char *from_args = NULL;
> > > + g_autofree char *to_args = NULL;
> > > + g_autofree char *exec_args = NULL;
> > > + g_auto(GStrv) argv = NULL;
> > > + char *from_str, *src, *dst;
> > > +
> > > + args->start.hide_stderr = false; /* omit redirection word from
> > > args */
> >
> > It's default off, right? Could I request for some more explanations?
>
> Yes, the default is false, so I will omit this line. I will change it to
> an assertion. (IIRC when I first wrote this code 1-2 years ago, the cpr-exec
> test was a derivative of a precopy common test that set hide_stderr=true).
>
> hide_stderr must be false when deriving cpr-exec arguments because of
> this code in framework.c:
>
> if (!getenv("QTEST_LOG") && args->hide_stderr) {
> ignore_stderr = "2>/dev/null";
>
> ignore_stderr is appended to the command line. For cpr-exec the command line
> may not include redirection, because we pass it to execv(), not to the shell.
Please kindly add this rich comment above the assertion..
>
> > Could we also set it in test_mode_exec() directly if needed?
>
> Yes, one can set hide_stderr when launching the source VM.
>
> > > + migrate_args(&from_args, &to_args, args->listen_uri, &args->start);
> > > + qtest_from_args = qtest_qemu_args(from_args);
> > > +
> > > + /* De-dup spaces so argv does not contain empty strings */
> > > + from_str = src = dst = g_strstrip(qtest_from_args);
> > > + do {
> > > + if (*src != ' ' || src[-1] != ' ') {
> > > + *dst++ = *src;
> > > + }
> > > + } while (*src++);
> >
> > Pure ask.. when will empty string be present?
>
> migrate_args() format strings "%s %s %s" produce " " when the arguments
> are empty strings. Then g_strsplit(" ") would produce an array of 3
> empty strings.
... and here too.
Thanks,
--
Peter Xu
