Re: [Qemu-devel] [PATCH RFC for-2.3 4/6] qga: implement qmp_guest_set_memory_blocks() for Linux with sysfs

[zhanghailiang <zhang.zhanghailiang@huawei.com>]

On 2014/12/22 5:23, Michael Roth wrote:
> Quoting zhanghailiang (2014-12-06 00:59:17)
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> ---
>>   qga/commands-posix.c | 42 +++++++++++++++++++++++++++++++++++-------
>>   1 file changed, 35 insertions(+), 7 deletions(-)
>>
>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>> index d3f7d4f..1010e86 100644
>> --- a/qga/commands-posix.c
>> +++ b/qga/commands-posix.c
>> @@ -2059,6 +2059,34 @@ GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
>>       error_propagate(errp, local_err);
>>       return NULL;
>>   }
>> +
>> +int64_t qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks,
>> +                                    Error **errp)
>> +{
>> +    int64_t processed;
>> +    Error *local_err = NULL;
>> +
>> +    processed = 0;
>> +    while (mem_blks != NULL) {
>> +        transfer_memory_block(mem_blks->value, false, &local_err);
>> +        if (local_err) {
>> +            break;
>> +        }
>
> The documentation claims to maximize the number of blocks we set in the event
> of an error, which would suggest we continue processing the remainder of the
> list rather than breaking here.
>

OK, one more thing, should we fix like that for set-vcpus ?

>> +        ++processed;
>> +        mem_blks = mem_blks->next;
>> +    }
>> +
>> +    if (local_err) {
>> +        if (processed == 0) {
>> +            error_propagate(errp, local_err);
>> +        } else {
>> +            error_free(local_err);
>> +        }
>
> Really think a list of error/status would make it easier for management to make
> sense of things.
>

Agreed :)

>> +    }
>> +
>> +    return processed;
>> +}
>> +
>>   #else /* defined(__linux__) */
>>
>>   void qmp_guest_suspend_disk(Error **errp)
>> @@ -2100,6 +2128,13 @@ GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
>>       return NULL;
>>   }
>>
>> +int64_t qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks,
>> +                                    Error **errp)
>> +{
>> +    error_set(errp, QERR_UNSUPPORTED);
>> +    return -1;
>> +}
>> +
>>   #endif
>>
>>   #if !defined(CONFIG_FSFREEZE)
>> @@ -2186,13 +2221,6 @@ GList *ga_command_blacklist_init(GList *blacklist)
>>       return blacklist;
>>   }
>>
>> -int64_t qmp_guest_set_memory_blocks(GuestMemoryBlockList *mem_blks,
>> -                                    Error **errp)
>> -{
>> -    error_set(errp, QERR_UNSUPPORTED);
>> -    return -1;
>> -}
>> -
>
> Same comment here as with prior patch
>

OK, thanks.

>>   int64_t qmp_guest_get_memory_block_size(Error **errp)
>>   {
>>       error_set(errp, QERR_UNSUPPORTED);
>> --
>> 1.7.12.4
>
>
> .
>