Peter Xu <[email protected]> writes:
> On Wed, Sep 02, 2026 at 07:15:38PM -0300, Fabiano Rosas wrote:
>> There are a couple of situations where all fields of a
>> MigrationParameters object need to be marked as present: when cloning
>> an entire object and when creating the transient object in
>> qmp_query_migrate(). The query-migrate-parameters QMP command contract
>> requires that all parameters, except block-bitmap-mapping, are present
>> in the output.
>>
>> Validate that a given object has all has_* fields set to true.
>>
>> Signed-off-by: Fabiano Rosas <[email protected]>
>> ---
>> migration/options.c | 54 +++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 54 insertions(+)
>>
>> diff --git a/migration/options.c b/migration/options.c
>> index bd7be8f9832..5d17acdd881 100644
>> --- a/migration/options.c
>> +++ b/migration/options.c
>> @@ -12,6 +12,7 @@
>> */
>>
>> #include "qemu/osdep.h"
>> +#include "qemu/cutils.h"
>> #include "qemu/error-report.h"
>> #include "qemu/units.h"
>> #include "exec/target_page.h"
>> @@ -23,8 +24,10 @@
>> #include "qapi/qmp/qerror.h"
>> #include "qapi/qobject-input-visitor.h"
>> #include "qapi/qobject-output-visitor.h"
>> +#include "qobject/qbool.h"
>> #include "qobject/qdict.h"
>> #include "qobject/qnull.h"
>> +#include "qobject/qstring.h"
>> #include "system/runstate.h"
>> #include "migration/colo.h"
>> #include "migration/cpr.h"
>> @@ -1149,12 +1152,63 @@ static void
>> migrate_mark_all_params_present(MigrationParameters *p)
>> }
>> }
>>
>> +static bool assert_all_params_present(MigrationParameters *params, Error
>> **errp)
>> +{
>> + g_autoptr(QDict) d = migrate_params_to_dict(params, errp);
>> + const QDictEntry *e = NULL;
>> + int i = 0;
>> +
>> + if (!d) {
>> + return false;
>> + }
>> +
>> + for (e = qdict_first(d); e; e = qdict_next(d, e), i++) {
>> + const char *key = qdict_entry_key(e);
>> + const char *p;
>> +
>> + if (strstart(key, "tls-", &p)) {
>> + QString *s = qobject_to(QString, qdict_entry_value(e));
>> +
>> + if (!s) {
>> + break;
>> + }
>> + } else if (strstart(key, "has-", &p)) {
>
> Does the qdict contain any has- field?
>
¬¬
I guess that decides the fate of the MigrationParameter enum.
> visit_type_MigrationParameters_members:
>
> if (visit_optional(v, "announce-initial", &obj->has_announce_initial)) {
> if (!visit_type_size(v, "announce-initial", &obj->announce_initial,
> errp)) {
> return false;
> }
> }
> ...
>
> It seems the has_* fields are only used to identify existance of objects,
> not converted.
>
>> + if (qdict_haskey(d, p)) {
>> + QBool *b = qobject_to(QBool, qdict_entry_value(e));
>> +
>> + If (!b || !qbool_get_bool(b)) {
>> + break;
>> + }
>> + }
>> + }
>> + }
>> +
>> + if (i && !e) {
>> + return true;
>> + }
>> +
>> + /*
>> + * Should never happen, but avoid asserting becase this is
>> + * reachable from QMP.
>
> IIUC as long as this fact shouldn't be changed by any possible user input,
> we could still assert. But I understand you want to be careful, maybe
> either (1) directly assert, or (2) change the function name,
> s/assert/check/? I vote (1).
>
> Said that, if the qdict trick didn't work it beats the whole patch.. so
> IMHO we can also leave this sanity check for later too. Your call.
>
>> + */
>> + error_setg(errp, "Missing parameter. Query output will be incomplete.");
>> + return false;
>> +}
>> +
>> MigrationParameters *qmp_query_migrate_parameters(Error **errp)
>> {
>> MigrationState *s = migrate_get_current();
>> MigrationParameters *params = QAPI_CLONE(MigrationParameters,
>> &s->parameters);
>>
>> + /*
>> + * Validate all parameters have their has_* field set to true as
>> + * consequence of the initial migrate_mark_all_params_present().
>> + */
>> + if (!assert_all_params_present(params, errp)) {
>> + return NULL;
>> + }
>> +
>> /*
>> * The block-bitmap-mapping breaks the expected API of
>> * query-migrate-parameters of having all members present. To keep
>> --
>> 2.53.0
>>
