From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Jackson Subject: Re: [PATCH v3 18/28] tools/libxl: Infrastructure to convert a legacy stream Date: Mon, 13 Jul 2015 16:44:10 +0100 Message-ID: <21923.56522.314560.338503@mariner.uk.xensource.com> References: <1436788907-1921-1-git-send-email-andrew.cooper3@citrix.com> <1436788907-1921-19-git-send-email-andrew.cooper3@citrix.com> <21923.53028.222582.402092@mariner.uk.xensource.com> <55A3D476.8040105@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55A3D476.8040105@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper Cc: Wei Liu , Ian Campbell , Xen-devel List-Id: xen-devel@lists.xenproject.org Andrew Cooper writes ("Re: [PATCH v3 18/28] tools/libxl: Infrastructure to convert a legacy stream"): > On 13/07/15 15:45, Ian Jackson wrote: ... > > I'm afraid I don't understand why this script needs to be told the > > toolstack build bit width. (If indeed the toolstack build bit width > > is the right thing, can't the script tell what bitness it is running > > on, some other way?) > > The script needs to know the bitness of the toolstack which saved the > legacy stream, not the bitness of the one which is currently running. But an ifdef tells you the bitness of the toolstack which is currently running. > The ifdefary above is an assumption for the common case. By > observation, noone outside of XenServer has tried migrating a VM from a > 32bit toolstack to a 64bit, or they have and not reported a bug. > Therefore, the common case is that the bitness of toolstack is the same. I see. > If in the future someone genuinely needs to convert legacy -> v2 and 32 > -> 64bit at the same time, they can use the conversion script manually > to achieve this (even in a migration), which will bypass the conversion > logic in libxl, as the incoming stream is already v2. How ... exciting. > > I appreciate that this seems very pernickety for compatibility code > > which will only be executed on i386 or amd64, but #ifdefs like this > > make the code hard to read IMO. > > Would it be acceptable to format the above description into a comment? That would help. But also I want rid of the #ifdef for stylistic reasons. How about you make the --width option optional, and default it to native bitness in the conversion script ? Ian.