qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Henrique Barboza <danielhb413@gmail.com>
To: qemu-devel@nongnu.org
Cc: dgilbert@redhat.com, kwolf@redhat.com, mreitz@redhat.com,
	armbru@redhat.com, muriloo@linux.ibm.com,
	Daniel Henrique Barboza <danielhb413@gmail.com>
Subject: [Qemu-devel] [PATCH v2 0/3] HMP/snapshot changes - do not use ID anymore
Date: Thu,  6 Sep 2018 08:11:04 -0300	[thread overview]
Message-ID: <20180906111107.30684-1-danielhb413@gmail.com> (raw)

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.


Daniel Henrique Barboza (3):
  block/snapshot.c: eliminate use of ID input in snapshot operations
  block/snapshot: remove bdrv_snapshot_delete_by_id_or_name
  qcow2-snapshot: remove redundant find_snapshot_by_id_and_name call

 block/qcow2-snapshot.c   |  5 -----
 block/snapshot.c         | 25 +++----------------------
 hmp-commands.hx          | 24 ++++++++++++------------
 include/block/snapshot.h |  3 ---
 qemu-img.c               | 15 +++++++++++----
 5 files changed, 26 insertions(+), 46 deletions(-)

-- 
2.17.1

             reply	other threads:[~2018-09-06 11:11 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-06 11:11 Daniel Henrique Barboza [this message]
2018-09-06 11:11 ` [Qemu-devel] [PATCH v2 1/3] block/snapshot.c: eliminate use of ID input in snapshot operations Daniel Henrique Barboza
2018-09-06 11:11 ` [Qemu-devel] [PATCH v2 2/3] block/snapshot: remove bdrv_snapshot_delete_by_id_or_name Daniel Henrique Barboza
2018-09-06 11:11 ` [Qemu-devel] [PATCH v2 3/3] qcow2-snapshot: remove redundant find_snapshot_by_id_and_name call Daniel Henrique Barboza
2018-10-08 18:13 ` [Qemu-devel] [PATCH v2 0/3] HMP/snapshot changes - do not use ID anymore Daniel Henrique Barboza
     [not found] ` <20180921122954.GD2842@work-vm>
     [not found]   ` <355a1147-c0d0-88fc-7b68-4391bab25c54@gmail.com>
2018-10-09 17:34     ` Markus Armbruster
2018-10-10  7:56       ` [Qemu-devel] [libvirt] " Peter Krempa
2018-10-11 12:06         ` Dr. David Alan Gilbert
2018-10-11 12:35           ` Peter Krempa
2019-01-09 14:10 ` [Qemu-devel] " Max Reitz
2019-01-09 14:21   ` Kevin Wolf
2019-01-09 14:27     ` Max Reitz
2019-01-09 14:48       ` Kevin Wolf
2019-01-09 14:54         ` Max Reitz
2019-01-09 15:13           ` Kevin Wolf
2019-01-09 15:25             ` Dr. David Alan Gilbert
2019-01-09 16:25             ` Markus Armbruster
2019-01-09 16:27             ` Max Reitz
2019-01-09 16:45               ` Kevin Wolf
2019-01-09 16:58                 ` Max Reitz
2019-01-09 18:19     ` Daniel Henrique Barboza
2019-01-09 16:57   ` Daniel Henrique Barboza
2019-01-09 17:05     ` Max Reitz
2019-01-09 17:20       ` Kevin Wolf
2019-01-09 17:38         ` Max Reitz
2019-01-09 17:52           ` Eric Blake
2019-01-09 19:00             ` Kevin Wolf
2019-01-11 12:35             ` Max Reitz
2019-01-09 17:55           ` Eric Blake
2019-01-09 18:51             ` Kevin Wolf
2019-01-09 19:02               ` Eric Blake
2019-01-10 11:25                 ` Kevin Wolf
2019-01-10 11:41                   ` Dr. David Alan Gilbert
2019-01-10 13:03                     ` Daniel Henrique Barboza
2019-01-10 15:11                     ` Kevin Wolf
2019-01-10 17:06                       ` Dr. David Alan Gilbert
2019-01-10 18:22                         ` Eric Blake
2019-01-11 12:14                           ` Kevin Wolf
2019-01-11 13:22                     ` Max Reitz
2019-01-11 14:33                       ` Kevin Wolf
2019-01-11 15:23                         ` Max Reitz
2019-01-09 17:32       ` Daniel Henrique Barboza
2019-01-09 17:07     ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180906111107.30684-1-danielhb413@gmail.com \
    --to=danielhb413@gmail.com \
    --cc=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=muriloo@linux.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).