From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Jackson Subject: Re: [PATCH v4 --for 4.6 COLOPre 11/25] tools/libxc: support to resume uncooperative HVM guests Date: Thu, 16 Jul 2015 16:40:25 +0100 Message-ID: <21927.53353.666566.702017@mariner.uk.xensource.com> References: <1436946351-21118-1-git-send-email-yanghy@cn.fujitsu.com> <1436946351-21118-12-git-send-email-yanghy@cn.fujitsu.com> <1436963167.32371.30.camel@citrix.com> <55A747B1.8070705@cn.fujitsu.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55A747B1.8070705@cn.fujitsu.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: Yang Hongyang Cc: wei.liu2@citrix.com, Ian Campbell , wency@cn.fujitsu.com, andrew.cooper3@citrix.com, yunhong.jiang@intel.com, eddie.dong@intel.com, xen-devel@lists.xen.org, guijianfeng@cn.fujitsu.com, rshriram@cs.ubc.ca List-Id: xen-devel@lists.xenproject.org Yang Hongyang writes ("Re: [Xen-devel] [PATCH v4 --for 4.6 COLOPre 11/25] tools/libxc: support to resume uncooperative HVM guests"): ... > This is used for secondary, at a checkpoint, we do: Thanks for this explanation, which helps somewhat. However, Ian Campbell asked for changes to the commit message to better explain what is going. I don't think what you have sent here is intended as a new commit message. So you aren't addressing his concern in the way that he is expecting. Ian wrote, in response to v3: I'm afraid I think the commit message for this patch (and the associated doc comments) need revisiting almost from scratch, to clearly explain what this patch is doing and why and what the constraints on the new functionality will be. At the moment it mostly talks in a confusing way about the old behaviour and adds very specific assumptions to the new function which are not made clear. To `revisit the commit message almost from scratch' means to completely rewrite the commit message. In v4 this patch had an additional paragraph in the commit message, but the commit message was otherwise substantially the same. So in response to v4 Ian C said: I'm afraid that the addition of [an extra] paragraph has not really addressed my comment on v3: and then he requoted the text above. Your response seems again to miss the main point of Ian's comment. If you are unable to rewrite the commit message right now because you don't understand what is missing or wrong, then we can discuss that more. We may even be able to help write it. However, it is not appropriate to simply ignore Ian Campbell's very clearly stated request for a complete overhaul of the commit message. While it is a good spirit of helpfulness to provide additional explanation in an email, that is not the same thing. Moving forward, what we need is a commit message which explains, in Ian Campbell's words: what this patch is doing That is, what the change in behaviour is. This includes clearly distinguishing old behaviour, before the patch, from new behaviour, after the patch. I appreciate that there may be language problems which are making this more difficult - I think your native language may not use tenses the way English does. So we can help you with the language, but we need the old and new behaviours to be clearly marked in your message. why You have already gone some way to answering this but the information needs to be folded into the commit message. what the constraints on the new functionality will be. It appears that you are supporting slow path resume for all HVM guests. Is that true ? Are there any cases left unhandled ? I suggest that the best thing for you to do next is either to reply to me with ideally (a) a draft of a new commit message for the patch, or if (as I suspect) you don't feel confident to do that, (a) questions to help you understand what we are looking for, or (c) a request for help with drafting. > While the XEN_DOMCTL_resumedomain hyper call for HVM is an NOP, it happens > to me that we could do this in a different way. We can modify > libxl__domain_resume, if the domain is HVM, we skip the xc_domain_resume > call, what do you think? Until we understand the answers to the questions above, it will be difficult for us to give a sensible opinion. Thanks, Ian.