From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:39866) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qgw6S-0006pp-Gg for qemu-devel@nongnu.org; Wed, 13 Jul 2011 05:51:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qgw6Q-00071e-KO for qemu-devel@nongnu.org; Wed, 13 Jul 2011 05:51:36 -0400 Received: from mail-gw0-f45.google.com ([74.125.83.45]:60156) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qgw6Q-0006yt-1S for qemu-devel@nongnu.org; Wed, 13 Jul 2011 05:51:34 -0400 Received: by gwb19 with SMTP id 19so2725846gwb.4 for ; Wed, 13 Jul 2011 02:51:33 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4E1C71F2.4030507@redhat.com> References: <4E131D0D.307@redhat.com> <20110711125432.GA19686@stefanha-thinkpad.localdomain> <20110711163226.GA10924@amt.cnet> <4E1C009C.1010408@redhat.com> <4E1C71F2.4030507@redhat.com> Date: Wed, 13 Jul 2011 10:51:32 +0100 Message-ID: From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] live block copy/stream/snapshot discussion List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Anthony Liguori , Dor Laor , Stefan Hajnoczi , Marcelo Tosatti , qemu-devel , Avi Kivity , Adam Litke On Tue, Jul 12, 2011 at 5:10 PM, Kevin Wolf wrote: > Am 12.07.2011 17:45, schrieb Stefan Hajnoczi: >>>>> The command synopses are as follows: >>>>> >>>>> block_stream >>>>> ------------ >>>>> >>>>> Copy data from a backing file into a block device. >>>>> >>>>> If the optional 'all' argument is true, this operation is performed i= n the >>>>> background until the entire backing file has been copied. =A0The stat= us of >>>>> ongoing block_stream operations can be checked with query-block-strea= m. >>> >>> Not sure if it's a good idea to use a bool argument to turn a command >>> into its opposite. I think having a separate command for stopping would >>> be cleaner. Something for the QMP folks to decide, though. >> >> git branch new_branch >> git branch -D new_branch >> >> Makes sense to me :) > > I don't think you should compare a command line option to a programming > interface. Having a git_create_branch(const char *name, bool delete) > would really look strange. Anyway, probably a matter of taste. > > A hint that separate commands would make sense is that the stop command > won't need the other arguments that the start command gets ('all' and > 'base'). I can see your point. Splitting the command might make the code more straightforward and eliminate the need for checking invalid argument combinations. >>>>> Arguments: >>>>> >>>>> - all: =A0 =A0copy entire device (json-bool, optional) >>>>> - stop: =A0 stop copying to device (json-bool, optional) >>>>> - device: device name (json-string) >>>> >>>> It must be possible to specify backing file that will be >>>> active after streaming finishes (data from that file will not >>>> be streamed into active file, of course). >>> >>> Yes, I think the common base image belongs here. >> >> Right. =A0We need to specify it by filename: >> >> =A0 - base: filename of base file (json-string, optional) >> >> =A0 Sectors are not copied from the base file and its backing file >> =A0 chain. =A0The following describes this feature: >> =A0 =A0 Before: base <- sn1 <- sn2 <- sn3 <- vm.img >> =A0 =A0 After: =A0base <- vm.img > > Does this imply that a rebase -u happens always after completion? Yes. The current implementation removes the backing file when streaming completes. I think this is the right thing to do since all sectors are now allocated - there is no way to use the backing file anymore. If we don't change the backing file on streaming completion, then the user has to issue an extra command. There's nothing to gain by doing that so I think rebase -u should happen on completion. >>> With all =3D false, where does the streaming begin? >> >> Streaming begins at the start of the image. >> >>> Do you have something like the "current streaming offset" in the state = of each BlockDriverState? >> >> Yes, there is a StreamState for each block device that has an >> in-progress operation. =A0The progress is saved between block_stream >> (without -a) invocations so the caller does not need to specify the >> streaming offset as an argument. >> >> Thanks for pointing out these weaknesses in the documentation. =A0It >> should really be explained fully. > > I think we also need to describe error cases. For example, what happens > if you try to start streaming while it's already in progress? Yes, will do. >>>>> Return: >>>>> >>>>> - device: device name (json-string) >>>>> - len: =A0 =A0size of the device, in bytes (json-int) >>>>> - offset: ending offset of the completed I/O, in bytes (json-int) >>> >>> So you only get the reply when the request has completed? With the >>> current monitor, this means that QMP is blocked while we stream, doesn'= t >>> it? How are you supposed to send the stop command then? >> >> Incomplete documentation again, sorry. =A0The block_stream command >> behaves as follows: >> >> 1. block_stream all returns immediately and the BLOCK_STREAM_COMPLETED >> event is raised when streaming completes either successfully or with >> an error. >> >> 2. block_stream stop returns when the in-progress streaming operation >> has been safely stopped. >> >> 3. block_stream returns when one iteration of streaming has completed. >> >>> Two of three examples below have an empty return value instead, so they >>> are not compliant to this specification. >> >> I will update the documentation, the non-all invocations do not return a= nything. > > Okay, then I don't understand what the 'offset' return value means. The > text says "offset of the completed I/O". If all=3Dtrue immediately > returns, shouldn't it always be 0? The 'offset' value gives you an indication of progress when using the iteration interface. You don't need to separately call query-block-stream, instead you can use the return value from the iteration interface to get progress information. However, let's drop iteration. >>> I find it rather disturbing that a command like 'change' has made it >>> into QMP... Anyway, I don't think this is really what we need. >>> >>> We have two switches to do. The first one happens before starting the >>> copy: Creating the copy, with the source as its backing file, and >>> switching to that. The monitor command to achieve this is snapshot_blkd= ev. >> >> I don't think that creating image files in QEMU is going to work when >> running KVM with libvirt (SELinux). =A0The QEMU process does not have >> the ability to create new image files. =A0It needs at least a file >> descriptor to an empty file or maybe a file that has been created >> using qemu-img like I showed above. > > Independent problem. We're really creating an external snapshot here, so > we should use the function for external snapshots. libvirt can > pre-create an empty image file, so that qemu will write the image format > data into it, but we have discussed this before. Cool. If snapshot_blkdev will be able to work in an sVirt environment then great. Stefan