From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MnvNs-000314-0t for qemu-devel@nongnu.org; Wed, 16 Sep 2009 10:21:24 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MnvNm-0002kI-6V for qemu-devel@nongnu.org; Wed, 16 Sep 2009 10:21:23 -0400 Received: from [199.232.76.173] (port=47706 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MnvNl-0002jy-Sn for qemu-devel@nongnu.org; Wed, 16 Sep 2009 10:21:17 -0400 Received: from an-out-0708.google.com ([209.85.132.240]:32009) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MnvNl-00057r-Jj for qemu-devel@nongnu.org; Wed, 16 Sep 2009 10:21:17 -0400 Received: by an-out-0708.google.com with SMTP id c38so1730996ana.37 for ; Wed, 16 Sep 2009 07:21:17 -0700 (PDT) Message-ID: <4AB0F45A.7000100@codemonkey.ws> Date: Wed, 16 Sep 2009 09:21:14 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: optional feature References: <20090916104620.GA4456@redhat.com> <20090916111845.GJ23157@redhat.com> <20090916115726.GL23157@redhat.com> <20090916123535.GM23157@redhat.com> <4AB0F17B.7000107@codemonkey.ws> <20090916141245.GC5287@redhat.com> In-Reply-To: <20090916141245.GC5287@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org, Gleb Natapov , Juan Quintela Michael S. Tsirkin wrote: > On Wed, Sep 16, 2009 at 09:08:59AM -0500, Anthony Liguori wrote: > >> Juan Quintela wrote: >> >>>>> up rtc version +1 >>>>> add the two fields that we need (together with rtc-td-hack value) >>>>> >>>>> >>>> And why this is better? You can't migrate old VM to new qemu even if you >>>> don't use rtc-td-hack on new one. >>>> >>>> >>> I think the difference between us is: >>> - is rtc-td-hack a hack that should only be used for some users >>> - it is a valid rtc feature that should be available to everybody >>> - it is independent, or it needs an rtc to have any value. >>> >>> >> We need a single table that contains the full state for the device. >> >> Many devices will have knobs. There are two likely types of knobs: >> >> 1) something that indicates how an array of state is going to be >> 2) a boolean that indicates whether a portion of state is valid >> >> rtc-td falls into the second category. It makes sense to me that the >> table state would contain a boolean to indicate whether a given set of >> state was valid. You may need a grouping mechanism for this. It >> probably makes sense to do this as separate tables. For instance, >> >> .fields = (VMStateField []) { >> VMSTATE_BOOL(td_hack, RTCState, (VMStateField[]){ >> VMSTATE_INT32(irq_coalesced, RTCState), >> VMSTATE_INT32(period, RTCState), >> VMSTATE_END_OF_LIST()}), >> } >> >> If we can't maintain backwards compatibility using this approach (we >> definitely can't for rtc-td) then we'll just have to live with that. >> > > We have to? Why do we? We could have an open loading function to load old versions of this device. It's ugly, but there's really no other way. > And not only won't we have backwards > compatibility now, we will also break it and have to break it each time > we add a knob. > No, we bump the version number and add more fields to the state. If we need to make crazy changes (like make a previously non-optional state, optional) then we can introduce two tables if we have to. > If instead we would only save/load the part of state if > the knob is set, we won't have a problem. > The rtc device happens to support an optional feature by splitting the optional bits into a separate section. Not every device does this though so if you want to convert other devices to this style, you'll break their backwards compatibility. The mechanisms are functionally the same. One is no more expressive than the other. It's the difference of vN introduces these optional features vs expliciting introducing new sections. What I don't like about the later is that these all need to be tied together in some sort of cohesive way. >> I also think arrays should be expressed like this FWIW. Today we have >> explicit typed arrays. I would rather see: >> >> .fields = (VMStateField []) { >> VMSTATE_ARRAY(nirq, PCIBus, (VMStateField[]) { >> VMSTATE_INT32(irq_count[0], PCIBus), >> VMSTATE_END_OF_LIST()}), >> } >> > > Same problem here. > I don't see what the problem is. Regards, Anthony Liguori