Re: [PATCH v4 --for 4.6 COLOPre 11/25] tools/libxc: support to resume uncooperative HVM guests

[Yang Hongyang <yanghy@cn.fujitsu.com>]

On 07/16/2015 11:40 PM, Ian Jackson wrote:
> 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.

I'm sorry, I didn't mean to it.

>
> 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.

I thought this is being addressed in the commit message, sorry again
for my poor English and not make it clear, I would appreciate your
help.

>
>    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 ?

For the first question, yes. For second, Sorry that I don't catch your question,
did you mean in some cases resuming HVM through slow path will be 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.

c, I would appreciate your help with drafting, thank you.

>
>
>> 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.
> .
>

-- 
Thanks,
Yang.