From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42670) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bkpZZ-0007C1-W9 for qemu-devel@nongnu.org; Fri, 16 Sep 2016 05:36:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bkpZX-00039a-8c for qemu-devel@nongnu.org; Fri, 16 Sep 2016 05:36:41 -0400 Received: from mail.ispras.ru ([83.149.199.45]:41474) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bkpZW-00036G-SD for qemu-devel@nongnu.org; Fri, 16 Sep 2016 05:36:39 -0400 From: "Pavel Dovgalyuk" References: <20160915090042.6440.22516.stgit@PASHA-ISP> <20160915090054.6440.77726.stgit@PASHA-ISP> <7d034a73-77d6-d79f-7b7b-137d8408e514@redhat.com> <001a01d20fef$b9a62700$2cf27500$@ru> In-Reply-To: Date: Fri, 16 Sep 2016 12:36:21 +0300 Message-ID: <002b01d20ffd$c911dfa0$5b359ee0$@ru> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Content-Language: ru Subject: Re: [Qemu-devel] [PATCH v2 02/10] block: set snapshot option for block devices in blkreplay module List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: 'Paolo Bonzini' , 'Pavel Dovgalyuk' , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, quintela@redhat.com, jasowang@redhat.com, mst@redhat.com, agraf@suse.de, david@gibson.dropbear.id.au > From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini > On 16/09/2016 09:55, Pavel Dovgalyuk wrote: > >> Since you have to create > >> overlay.qcow2 outside QEMU anyway, overlay.qcow2 might as well be the > >> "image". That is, you could choose between: > >> > >> -drive driver=blkreplay,if=none,image=overlay.qcow2,id=img-blkreplay \ > >> -rr snapshot=replay_init,... > >> > >> -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay > >> > >> The temporary snapshot would be created if there's no "-rr snapshot" option > >> on the command line. > >> > >> Does this make sense? > > > > There are two different parts: > > - creating an overlay > > - creating the snapshot > > > > Overlay is needed to preserve the state of the original backing file. > > In the current version temporary overlay is always created at start of qemu. > > Yes, this would still be the default for rr mode. > > > I don't think that it is convenient forcing user to create overlay manually. > > Note that all I'm only saying that _only for the case where the user > creates the overlay manually anyway_ there is no need to specify both > image and overlay. (I also don't like particularly the hard-coded > snapshot name replay_init, which can be overridden by -loadvm but not > when saving). Ok, this seems reasonable to fix. > > So there are various possibilites: > > First proposal: > > - automatically created overlay is -icount rr=record|replay (then > snapshot name doesn't matter, it can be replay_init) > > - manually created overlay is -icount > rr=record|replay,rrsnapshot=snapname (then snapshot name matters because > you can have different snapshots in the same file) We can't create overlay with icount suboptions, because there could be several block devices. Each one needs its own overlay. > If rr is enabled but rrsnapshot is absent, configure_icount can just set > "snapshot = 1" to force creation of the temporary overlay. This > requires no change to the blkreplay driver > > > Second proposal: > > - automatically created overlay is -icount rr=record|replay -snapshot > > - manually created overlay is -icount rr=record|replay and an rrsnapshot > suboption can be added anyway if considered useful. See above. > This requires no change to the blkreplay driver either. It's a little > more verbose in the common case, but perhaps less surprising if you're > already used to -snapshot. Our internal version automatically creates overlays with generated names. But now I think, that it is inconvenient and manual name specification is better. > > Common debugging scenario includes multiple recording passes until the bug manifests > > itself. Every new execution recorded should be accompanied by creating an overlay > > to assure that the execution is started from the same disk state. > > > > Specifying initial snapshot name makes sense if we want to suppress -loadvm option. > > My rationale was to have similar command lines between record and replay > modes (just changing rr=record to rr=replay). This seems reasonable, I'll try to fix this part. Pavel Dovgalyuk