From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34108) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Srqc9-0007Lj-OB for qemu-devel@nongnu.org; Thu, 19 Jul 2012 09:17:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Srqc3-0000R4-6r for qemu-devel@nongnu.org; Thu, 19 Jul 2012 09:17:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:10687) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Srqc2-0000MJ-Tx for qemu-devel@nongnu.org; Thu, 19 Jul 2012 09:17:51 -0400 Date: Thu, 19 Jul 2012 10:18:21 -0300 From: Luiz Capitulino Message-ID: <20120719101821.48c822bf@doriath.home> In-Reply-To: <5007C286.6020400@redhat.com> References: <1339598189-17933-1-git-send-email-wdongxu@linux.vnet.ibm.com> <1339598189-17933-5-git-send-email-wdongxu@linux.vnet.ibm.com> <4FD9C40C.1@redhat.com> <4FD9C8A0.6080504@redhat.com> <4FD9CC26.3040306@redhat.com> <5007C286.6020400@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 5/6] add-cow: support snapshot_blkdev List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Paolo Bonzini , Dong Xu Wang , qemu-devel@nongnu.org, Stefan Hajnoczi On Thu, 19 Jul 2012 10:17:10 +0200 Kevin Wolf wrote: > Am 19.07.2012 04:20, schrieb Dong Xu Wang: > > On Thu, Jun 14, 2012 at 7:33 PM, Kevin Wolf wrote: > >> Am 14.06.2012 13:18, schrieb Paolo Bonzini: > >>> Il 14/06/2012 12:59, Kevin Wolf ha scritto: > >>>> Am 13.06.2012 16:36, schrieb Dong Xu Wang: > >>>>> add-cow will let raw file support snapshot_blkdev indirectly. > >>>>> > >>>>> Signed-off-by: Dong Xu Wang > >>>> > >>>> Paolo, what do you think about this magic? > >>> > >>> Besides the obvious layering violation (it would be better to add a new > >>> method bdrv_ext_snapshot_create perhaps) I don't see very much a problem > >>> in it. Passing image creation options sounds like a good idea that we > >>> can add in the future as an extension. > >>> > >>> But honestly, I don't really see the point of add-cow in general... The > >>> raw image is anyway not usable without a pass through qemu-io convert, > >>> and mirroring will also allow collapsing an image to raw (with the > >>> persistent dirty bitmap playing the role of the add-cow metadata). > >> > >> Well, the idea was that you stream into add-cow and once the streaming > >> has completed, you can just drop the bitmap. > >> > >> There might be some overlap with mirroring, though when we discussed > >> introducing add-cow, mirrors were not yet on the table. One difference > >> that remains is that with streaming the guest only writes to the target > >> image and can't have any problem with convergence. > >> > >>>> I think I can see its use especially for HMP because it's quite > >>>> convenient, but on the other hand it assumes a fixed image path for the > >>>> new raw image. This isn't going to work for block devices, for example. > >>> > >>> True, but then probably you would use mode='existing', because you need > >>> to pre-create the logical volume. > >> > >> I think it might be convenient to have the raw volume precreated (you > >> need to do that anyway), but create the COW bitmap only during the > >> snapshot command. But yeah, not really important. > >> > >>>> If we don't do it this way, we need to allow passing image creation > >>>> options to the snapshotting command so that you can pass a precreated > >>>> image file. > >>> > >>> This sounds like a useful extension anyway, except that passing an > >>> unstructured string for image creation options is ugly... Perhaps we > >>> can base a better implementation of options on Laszlo's QemuOpts visitor. > >> > >> Yes, I wouldn't want to use a string here, we should use something > >> structured. Image creation still uses the old-style options instead of > >> QemuOpts, but maybe this is the opportunity to convert it. > > > > Kevin, do you mean I need to replace QEMUOptionParameter with QemuOpts? > > > > If true, other image formats should also be changed, I noticed Stefan > > has an un-comleted patch: > > http://repo.or.cz/w/qemu/stefanha.git/commitdiff/b49babb2c8b476a36357cfd7276ca45a11039ca5 > > > > then I can work based on Stefan's patch. > > Yes, the direction of this patch from Stefan's repo looks good. > > I'm not sure whether it will immediately allow passing structured image > creation options in QMP, though. It might work using the old-style > monitor commands, just getting a QDict from the options. Not quite sure > how this is going to work with QAPI. We did it for netdev_add and it works as you said above. The schema entry looks like this: { 'command': 'netdev_add', 'data': {'type': 'str', 'id': 'str', '*props': '**'}, 'gen': 'no' } Where 'props' is: # @props: #optional a list of properties to be passed to the backend in # the format 'name=value', like 'ifname=tap0,script=no' Then we have the qmp front-end, using the old style monitor signature: int qmp_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret); And we have the HMP front-end (which should be used elsewhere too): void netdev_add(QemuOpts *opts, Error **errp); As Paolo said, Laszlo's work will help us to this the right way.