Re: [Qemu-devel] [PATCH v4 02/11] block: add error parameter to del_existing_snapshots()

[Markus Armbruster]

Pavel Hrdina <address@hidden> writes:

> On 9.4.2013 15:27, Markus Armbruster wrote:
>> Pavel Hrdina <address@hidden> writes:
>>
>>> Signed-off-by: Pavel Hrdina <address@hidden>
>>> Reviewed-by: Eric Blake <address@hidden>
>>> ---
>>>   savevm.c | 12 +++++++-----
>>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/savevm.c b/savevm.c
>>> index 77c5291..dc1f4a4 100644
>>> --- a/savevm.c
>>> +++ b/savevm.c
>>> @@ -2224,7 +2224,7 @@ static int bdrv_snapshot_find(BlockDriverState *bs, 
>>> QEMUSnapshotInfo *sn_info,
>>>   /*
>>>    * Deletes snapshots of a given name in all opened images.
>>>    */
>>> -static int del_existing_snapshots(Monitor *mon, const char *name)
>>> +static int del_existing_snapshots(const char *name, Error **errp)
>>>   {
>>>       BlockDriverState *bs;
>>>       QEMUSnapshotInfo sn1, *snapshot = &sn1;
>>> @@ -2237,9 +2237,8 @@ static int del_existing_snapshots(Monitor *mon, const 
>>> char *name)
>>>           {
>>>               ret = bdrv_snapshot_delete(bs, name);
>>>               if (ret < 0) {
>>> -                monitor_printf(mon,
>>> -                               "Error while deleting snapshot on '%s'\n",
>>> -                               bdrv_get_device_name(bs));
>>> +                error_setg(errp, "error while deleting snapshot on '%s'",
>>> +                           bdrv_get_device_name(bs));
>>>                   return -1;
>>>               }
>>>           }
>>
>> Any particular reason for de-capitalizing "Error"?
>
> Yes, Eric told me that we should start the error message with lover
> case and also we shouldn't print '.' at the end of the error message.

I agree on not ending error messages with a period.  Whether to start
them with a capital letter or not isn't worth arguing over, given the
general state of our error reporting.

[...]