[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 08/11] dump-guest-memory: add qmp event DUMP_
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v4 08/11] dump-guest-memory: add qmp event DUMP_COMPLETED |
Date: |
Wed, 2 Dec 2015 07:45:52 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 12/01/2015 06:11 PM, Fam Zheng wrote:
> On Tue, 12/01 21:28, Peter Xu wrote:
>> One new QMP event DUMP_COMPLETED is added. When a dump finishes, one
>> DUMP_COMPLETED event will occur to notify the user.
>>
>> Signed-off-by: Peter Xu <address@hidden>
>> ---
>> +++ b/qapi/event.json
>> @@ -356,3 +356,16 @@
>> ##
>> { 'event': 'MEM_UNPLUG_ERROR',
>> 'data': { 'device': 'str', 'msg': 'str' } }
>> +
>> +##
>> +# @DUMP_COMPLETED
>> +#
>> +# Emitted when background dump has completed
>> +#
>> +# @error: #optional human-readable error string that provides
>> +# hint on why dump failed.
>
> Please explicitly mention that successful dump emits DUMP_COMPLETED without
> error, and failed dump emits DUMP_COMPLETED that has an error str.
In fact, I wonder if it would also be worth having a
'status':'DumpStatus' field, which records the final status of the dump
(either 'completed' or 'failed'), and which is always present.
>> +++ b/util/error.c
>> @@ -197,7 +197,11 @@ ErrorClass error_get_class(const Error *err)
>>
>> const char *error_get_pretty(Error *err)
>> {
>> - return err->msg;
>> + if (err) {
>> + return err->msg;
>> + } else {
>> + return NULL;
>> + }
>
> This change belongs to a separate patch, if any.
Indeed. When I was musing about the idea, I was not expecting you to
actually implement it, so much as questioning whether it is a worthwhile
idea. But as it impacts more than just your series, it definitely needs
to be a separate patch, if at all.
> But personally I don't like
> it, because it doesn't work very well when error_get_pretty is used in
> printf-like function parameters:
>
> Error *err = NULL;
> error_report("error: %s", error_get_pretty(err));
>
> will print "error: (null)" which is ugly,
Or even segfault. glibc is nice for printing "(null)", but the behavior
is undefined by POSIX and other libc aren't as nice as glibc. And that
was not a consequence I thought about when first raising the question of
whether it was even worth changing the contract of error_get_pretty().
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v4 05/11] dump-guest-memory: introduce dump_process() helper function., (continued)
[Qemu-devel] [PATCH v4 09/11] DumpState: adding total_size and written_size fields, Peter Xu, 2015/12/01
- Re: [Qemu-devel] [PATCH v4 09/11] DumpState: adding total_size and written_size fields, Fam Zheng, 2015/12/01
- Re: [Qemu-devel] [PATCH v4 09/11] DumpState: adding total_size and written_size fields, Peter Xu, 2015/12/02
- Re: [Qemu-devel] [PATCH v4 09/11] DumpState: adding total_size and written_size fields, Fam Zheng, 2015/12/02
- Re: [Qemu-devel] [PATCH v4 09/11] DumpState: adding total_size and written_size fields, Peter Xu, 2015/12/02
- Re: [Qemu-devel] [PATCH v4 09/11] DumpState: adding total_size and written_size fields, Fam Zheng, 2015/12/02
- Re: [Qemu-devel] [PATCH v4 09/11] DumpState: adding total_size and written_size fields, Peter Xu, 2015/12/02