From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53054) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VKSdB-0005UB-58 for qemu-devel@nongnu.org; Fri, 13 Sep 2013 08:37:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VKSd5-0008Bo-4s for qemu-devel@nongnu.org; Fri, 13 Sep 2013 08:37:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:64396) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VKSd4-0008Bi-T8 for qemu-devel@nongnu.org; Fri, 13 Sep 2013 08:37:43 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r8DCbgUb024625 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 13 Sep 2013 08:37:42 -0400 Message-ID: <52330713.4050604@redhat.com> Date: Fri, 13 Sep 2013 14:37:39 +0200 From: Max Reitz MIME-Version: 1.0 References: <1378106712-29856-1-git-send-email-mreitz@redhat.com> <5231D673.6050907@redhat.com> <5231DCA6.7090004@redhat.com> <20130913095753.GC2804@dhcp-200-207.str.redhat.com> <5232E78A.2030406@redhat.com> <52330516.10503@redhat.com> In-Reply-To: <52330516.10503@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 0/8] Add metadata overlap checks List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi On 2013-09-13 14:29, Eric Blake wrote: > On 09/13/2013 04:23 AM, Max Reitz wrote: >>> The more interesting part is that adding an option always needs thought >>> because once it is exposed, it's an API that is set in stone. And I'm >>> also not sure what the best command line and QMP representations of a >>> bitmask like this are. >> I'd personally add it to the runtime options of qcow2. In addition, I >> propose we add a mechanism to generally amend runtime options at runtime >> through QMP (if there isn't one already which I then am unaware of). I >> don't see why we should just allow the kind of overlap checks performed >> to be changed at runtime, but not, for instance, whether lazy refcounts >> should be used (except the latter would be a bit harder to implement, I >> guess). > Indeed - I was asking more to spark conversation, and not to necessarily > state that we need it (again, without benchmark numbers, it's hard to > state whether there's enough timing difference for it to even matter > that someone would WANT a runtime tuning). IF we implement runtime > tuning, we're stuck supporting it. I have done some short tests here (I think just writing some GB of data to a qcow2 image on a ramdisk should suffice; maybe taking a couple of snapshots to increase the number of inactive L1 tables (although the active L2 tables should affect the results most anyway)) and I don't see a difference so far. I will continue those tests at home where I have more RAM. >> About the representation: The discard behavior is basically a bitfield >> already and gives us therefore one possible representation (which is, >> just using a single boolean per structure, named something like >> "overlap-check.active-l1" etc.). In QMP we could probably also use a >> dict, but then again, this is a decision to be made when generally >> allowing modification of the qcow2 runtime options through QMP (in my >> opinion). And finally, we could obviously just use an integer to >> represent the mask. > Implement it only as a struct of bools. A raw 'int' requires the caller > to have too much internal knowledge of which bools map to which bit > positions, and furthermore prevents you from ever changing bit positions. > >> I think, we should first take care of the command line interface and >> about QMP later (that is, if you agree on generally allowing >> modification of the qcow2 runtime options through QMP). There, we could >> offer both one boolean per mask element and an integer option, probably >> the boolean flags taking precedence. > I'm fine if it is JUST a command-line parameter (all-or-nothing, turned > on when you boot qemu, and not something we can be changing on the fly). > But if we ever do want live changing via QMP, do NOT expose it as a raw > int, but only as named bools. > >> The flags are nice for users who want an "easily" comprehensible >> interface, the masked integer is better for those who prefer a short >> representation. > Short representations that lock us into a particular implementation are bad. > Yes, that's true. Max