[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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


     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.



Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.