Re: [Qemu-devel] [PATCH v2 6/8] migration: Add multifd-compress parameter

[Juan Quintela]

Markus Armbruster <address@hidden> wrote:
> Juan Quintela <address@hidden> writes:
>
>> Signed-off-by: Juan Quintela <address@hidden>
>>
>> ---
>> Rename it to NONE

>> @@ -1822,6 +1826,19 @@ void hmp_migrate_set_parameter(Monitor *mon, const 
>> QDict *qdict)
>>          p->has_multifd_channels = true;
>>          visit_type_int(v, param, &p->multifd_channels, &err);
>>          break;
>> +    case MIGRATION_PARAMETER_MULTIFD_COMPRESS:
>> +        p->has_multifd_compress = true;
>> +        visit_type_enum(v, param, &compress_type,
>> +                        &MultifdCompress_lookup, &err);
>
> visit_type_MultifdCompress(), please.

done.

Interesting that I can

#include "qapi/qapi-visit-common.h"

but not what I would expect/want:

#include "qapi/qapi-visit.h"

Perhaps we should remove

#include "qapi-visit-target.h"

from there?

Anyways, independent of this patch.

>> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
>> index 5da1439a8b..7c8e71532f 100644
>> --- a/hw/core/qdev-properties.c
>> +++ b/hw/core/qdev-properties.c
>> @@ -645,6 +645,17 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
>>      .set_default_value = set_default_value_enum,
>>  };
>>  
>> +/* --- MultifdCompress --- */
>> +
>> +const PropertyInfo qdev_prop_multifd_compress = {
>> +    .name = "MultifdCompress",
>> +    .description = "multifd_compress values",
>
> Similar property declarations list the valid values in .description.

Fixed, thanks.

>>  
>> +#define DEFINE_PROP_MULTIFD_COMPRESS(_n, _s, _f, _d) \
>> +    DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_multifd_compress, 
>> MultifdCompress)
>> +
>
> As long as qdev_prop_multifd_compress is exposed in qdev-properties.h,
> hiding the macro using it doesn't make sense to me.

Where do you want it to be?  This should only be used here, but if you
want it anywhere else, told me where.


>> +##
>> +# @MultifdCompress:
>> +#
>> +# An enumeration of multifd compression.
>> +#
>> +# @none: no compression.
>> +#
>> +# Since: 4.1
>> +#
>> +##
>> +{ 'enum': 'MultifdCompress',
>> +  'data': [ 'none' ] }
>
> Any particular reason for putting this in common.json?  As is, it looks
> rather migration-specific to me...

Not sure if with new "qapi" compiler it works, it used to be that it
failed if you declared an enum anywhere else.  See how I have to put
property info into qdev-properties.c instead of any migration file.


Thanks, Juan.