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

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel