From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 08/18] PVH xen: tools changes to create PVH domain Date: Thu, 29 Aug 2013 10:01:25 +0100 Message-ID: <1377766885.11455.15.camel@kazak.uk.xensource.com> References: <1369445137-19755-1-git-send-email-mukesh.rathor@oracle.com> <1369445137-19755-9-git-send-email-mukesh.rathor@oracle.com> <1371049088.24512.450.camel@zakaz.uk.xensource.com> <20130614171437.49f55cea@mantra.us.oracle.com> <1371467494.23802.49.camel@zakaz.uk.xensource.com> <20130730164716.10969419@mantra.us.oracle.com> <1375272057.7382.24.camel@kazak.uk.xensource.com> <20130731190213.0b57efd0@mantra.us.oracle.com> <1375344091.15681.41.camel@dagon.hellion.org.uk> <20130828185152.0f7bcf8c@mantra.us.oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20130828185152.0f7bcf8c@mantra.us.oracle.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: Mukesh Rathor Cc: "Xen-devel@lists.xensource.com" , Ian Jackson List-Id: xen-devel@lists.xenproject.org On Wed, 2013-08-28 at 18:51 -0700, Mukesh Rathor wrote: > On Thu, 1 Aug 2013 09:01:31 +0100 > Ian Campbell wrote: > > > On Wed, 2013-07-31 at 19:02 -0700, Mukesh Rathor wrote: > > > On Wed, 31 Jul 2013 13:00:57 +0100 > > > Ian Campbell wrote: > > > > > > > On Tue, 2013-07-30 at 16:47 -0700, Mukesh Rathor wrote: > > > > > On Mon, 17 Jun 2013 12:11:34 +0100 > > > > > Ian Campbell wrote: > ... > > I think it is valid for a user to both request PVH mode and request > > some features of their own which they want to enable for this guest. > > The code should be set up to deal with this, even if that currently > > means rejecting any user request for a feature not in the PVH set > > (although I'd prefer to see a stronger rationale for rejecting a > > requested feature than that). > > > > > I can juse use the existing > > > xc_interface_core.flags? (would like to rename it to xc_flags so > > > one can easily find its usages please :)). > > > > Pointless churn. You've made this argument countless times about > > various and been told no by several different people, please just > > drop it. > > > > > So: > > > > > > xc_dom_allocate: > > > > > > if (xch->flags & PVH) > > > { > > > if (features) > > > { > > > error > > > return NULL; > > > > No, please handle the case of the user asking for features. > > > > At the very least if they only ask for things in the set you are > > forcing then it is fine. > > > > > } > > > features = > > > writable_descriptor_tables|auto_translated_physmap" > > > "|supervisor_mode_kernel|hvm_callback_vector; > > > > Don't do this. Instead, drop this whole if and add below: > > > > > } > > > if ( features ) > > > elf_xen_parse_features(features, dom->f_requested, NULL); > > > > if ( xch->flags & PVH ) > > elf_xen_parse_features("writable_desc...|etc", > > dom->f_requested, dom->f_required) > > Hmm.. the problem I am running here now is setting of PVH flag in > xch->flags from libxl? struct xch seems to be private to libxc. xch is the libxc handle used by all the api calls, so it can't be private to libxc. There is an xch inside the libxl ctx, use either ctx->xch or CTX->xch depending on whether you have a ctx or a gc in the function in question. Actually, xch->flags & PVH is not the right place. xch is a handle onto an open libxc instance, it is not per-domain, so adding PVH to xch->flags is wrong. Not sure how I missed that initially. I think you need to add the flag to the dom->flags in libxl__build_pv. I don't think anything before the existing setting of that field needs to know if the guest is PVH or not. The calls between xc_dom_allocate and there are xc_dom_(kernel|ramdisk)_(file|mem) which are just setting up internal state and not touching the guest yet. If I'm wrong about that then I think the block setting all of those dom->fields can be moved up. Ian.