Re: [Qemu-devel] [PATCH v4 00/11] convert savevm to use qapi and introduce qmp command

[Pavel Hrdina <phrdina@redhat.com>]

On 10.4.2013 15:32, Luiz Capitulino wrote:
> On Wed, 10 Apr 2013 15:22:49 +0200
> Pavel Hrdina <phrdina@redhat.com> wrote:
>
>> On 10.4.2013 14:49, Eric Blake wrote:
>>> On 04/10/2013 06:40 AM, Luiz Capitulino wrote:
>>>> On Wed, 10 Apr 2013 06:24:11 -0600
>>>> Eric Blake <eblake@redhat.com> wrote:
>>>>
>>>>>>     - If you want to overwrite an existing snapshot, you could specify
>>>>>>       the 'id' or the 'name' argument or both of them and also you will
>>>>>>       have to use the 'force' argument
>>>>>
>>>>> But the argument made in this thread is that QMP should _not_ have a
>>>>> force argument.  It should be a flat-out error in QMP to try to create a
>>>>> snapshot with a conflicting name or tag; preferably with a distinct
>>>>> error type.  Higher-level apps (HMP savevm -f) would try to create; if
>>>>> -f is not specified, the error is good enough; if -f is specified and
>>>>> that particular error is returned, then HMP calls delete and then
>>>>> re-tries the create.  No 'force' argument needed at the QMP layer.
>>>>
>>>> To avoid adding a new error class, the HMP command could query for the
>>>> snapshot name and delete it if it exists before creating the snapshot.
>>>
>>> Atomic collision detection is nicer than having to pre-query - fewer QMP
>>> calls in the common case.  But you're right that none of the existing
>>> ErrorClass categories fit the idea of "creation refused because name
>>> would collide".
>
> We can add a new error class if it's important to be atomic. I don't think
> this is hugely important though, first because we don't support multiple
> QMP connections (it should work, but we don't advertise it as supported) and
> also because I don't think we have ever designed commands with this in mind.
>
>> We still could have the force parameter for HMP because the QMP command
>> will return an error message that the snapshot already exist. The only
>> drawback will be that the error message will not contain an information
>> that you could use a '-f' parameter for override.
>
> You're right, but we're discussing how the HMP command could detect that
> an existing snapshot should be deleted.
>

If you will get an error about existing snapshot then you could write
"savevm -f my_name" and the code could be something like this:

     if (force) {
         qmp_vm_snapshot_delete(!!name, name, false, 0, &local_err);
         if (error_is_set(&local_err) {
             hmp_handle_error(mon, &local_err);
             return;
         }
     }

     info = qmp_vm_snapshot_save(!!name, name, &local_err);

     if (info) {
         print_info
     }

     qapi_free_SnapshotInfo(info);
     hmp_handle_error(mon, &local_err);

The only requirement is that the qmp_vm_snapshot_delete cannot set an 
error if the snapshot doesn't exist. To get this information the 
qmp_vm_snapshot_delete will return nothing or SnapshotInfo of deleted 
snapshot.