[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v5 6/9] libxc: create unmapped initrd in domain builder if supported
On 30/11/15 12:23, Ian Campbell wrote: > On Mon, 2015-11-30 at 12:03 +0100, Juergen Gross wrote: >> On 30/11/15 11:52, Ian Campbell wrote: >>> On Mon, 2015-11-30 at 10:51 +0000, Ian Campbell wrote: >>>> On Mon, 2015-11-30 at 11:47 +0100, Juergen Gross wrote: >>>>> On 30/11/15 11:34, Ian Campbell wrote: >>>>>> On Mon, 2015-11-30 at 11:23 +0100, Juergen Gross wrote: >>>>>>> On 30/11/15 11:20, Wei Liu wrote: >>>>>>>> On Thu, Nov 26, 2015 at 08:35:02AM +0100, Juergen Gross >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> /* initrd parameters as specified in start_info page >>>>>>>>> */ >>>>>>>>> - unsigned long initrd_start; >>>>>>>>> - unsigned long initrd_len; >>>>>>>>> + uint64_t initrd_start; >>>>>>>>> + uint64_t initrd_len; >>>>>>>>> >>>>>>>> >>>>>>>> I think these should be of type xen_vaddr_t. Doesn't make a >>>>>>>> difference >>>>>>>> in the end though. >>>>>>> >>>>>>> xen_vaddr_t seems not to be appropriate. It can be either a >>>>>>> virtual >>>>>>> address or a pfn. >>>>>> >>>>>> Did you mean a virtual address or a physical _address_? >>>>>> Potentially >>>>>> mixing >>>>>> addresses and frame numbers in a single variable seems liable to >>>>>> be >>>>>> confusing, at best. >>>>> >>>>> No, it's really a pfn. And this is part of the stable interface >>>>> between >>>>> hypervisor and the pv-domU since more than 5 years now. >>>> >>>> Including the virtual address bit? >>>> >>>> That's a shame. >>> >>> ... and that being the case would you mind adding a comment here >>> explaining >>> the two forms of these variables and the flag which indicates which one >>> is >>> "in force" at a given moment. >> >> The comment in the struct already tells us that initrd_start and >> initrd_len are in the very same format as in the start_info page. >> Both fields are meant to be opaque to most of the domain builder >> parts. >> >> The only function dealing with the differences is xc_dom_build_image() >> which already contains the appropriate flag. I added this on your >> request. You acked the resulting patch. So why do you want to add >> another comment now? > > I hadn't realised at the time that the semantics of these fields was so, > uh, interesting. :-) I guess due to the lack of a comment? ;-) Okay, I'll add one when submitting the patch after (hopefully) Boris confirmed it is fixing his problem. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |