From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:52879) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ghHAl-000681-Mg for qemu-devel@nongnu.org; Wed, 09 Jan 2019 11:57:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ghHAk-0006Sl-Kl for qemu-devel@nongnu.org; Wed, 09 Jan 2019 11:57:43 -0500 Received: from mail-qk1-x741.google.com ([2607:f8b0:4864:20::741]:40343) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1ghHAk-0006SH-G8 for qemu-devel@nongnu.org; Wed, 09 Jan 2019 11:57:42 -0500 Received: by mail-qk1-x741.google.com with SMTP id y16so4803022qki.7 for ; Wed, 09 Jan 2019 08:57:41 -0800 (PST) References: <20180906111107.30684-1-danielhb413@gmail.com> <47023eb5-41f1-1b60-1094-d607999e93b6@redhat.com> From: Daniel Henrique Barboza Message-ID: Date: Wed, 9 Jan 2019 14:57:37 -0200 MIME-Version: 1.0 In-Reply-To: <47023eb5-41f1-1b60-1094-d607999e93b6@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Subject: Re: [Qemu-devel] [PATCH v2 0/3] HMP/snapshot changes - do not use ID anymore List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-devel@nongnu.org Cc: dgilbert@redhat.com, kwolf@redhat.com, armbru@redhat.com, muriloo@linux.ibm.com On 1/9/19 12:10 PM, Max Reitz wrote: > On 06.09.18 13:11, Daniel Henrique Barboza wrote: >> changes in v2: >> - removed the "RFC" marker; >> - added a new patch (patch 2) that removes >> bdrv_snapshot_delete_by_id_or_name from the code; >> - made changes in patch 1 as suggested by Murilo; >> - previous patch set link: >> https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg04658.html >> >> >> It is not uncommon to see bugs being opened by testers that attempt to >> create VM snapshots using HMP. It turns out that "0" and "1" are quite >> common snapshot names and they trigger a lot of bugs. I gave an example >> in the commit message of patch 1, but to sum up here: QEMU treats the >> input of savevm/loadvm/delvm sometimes as 'ID', sometimes as 'name'. It >> is documented as such, but this can lead to strange situations. >> >> Given that it is strange for an API to consider a parameter to be 2 fields >> at the same time, and inadvently treating them as one or the other, and >> that removing the ID field is too drastic, my idea here is to keep the >> ID field for internal control, but do not let the user set it. >> >> I guess there's room for discussion about considering this change an API >> change or not. It doesn't affect users of HMP and it doesn't affect Libvirt, >> but simplifying the meaning of the parameters of savevm/loadvm/delvm. > (Yes, very late reply, I'm sorry...) > > I do think it affects users of HMP, because right now you can delete > snapshots with their ID, and after this series you cannot. That's true. My idea here was simple: the user can't reliably exclude via snapshot ID today because we're hiding the ID field in info snapshots:     (qemu) savevm 0     (qemu) info snapshots     List of snapshots present on all disks:     ID        TAG                 VM SIZE                DATE VM CLOCK     --        0                      741M 2018-07-31 13:39:56 00:41:25.313 Thus, what will end up happening is that the user will be forced to use the TAG of the snapshot since this is the only available information. > > I think we had a short discussion about just disallowing numeric > snapshot names. How bad would that be? This was my first idea when evaluating what to do in this case. I gave it up because I found it to be too extreme. People would start complaining "I was able to do savevm 0 and now I can't". > > Max >