Re: [Qemu-devel] Re: KVM call minutes for June 15

2010-06-16 Thread Markus Armbruster
Anthony Liguori anth...@codemonkey.ws writes:

 On 06/15/2010 10:41 AM, Christoph Hellwig wrote:
 On Tue, Jun 15, 2010 at 08:18:12AM -0700, Chris Wright wrote:

 KVM/qemu patches
 - patch rate is high, documentation is low, review is low
 - patches need to include better descriptions and documentation
- will slow down patch writers
- will make it easier for patch reviewers
  
 What is the qemu patch review policy anyway?

 We don't really have a coherent policy.  Suggestions for improvement
 are always appreciated.

There are no
 Reviewed-by: included in the actual commits,

 Reviewed-by/Ack-by's are pretty helpful for me.  In terms of including
 them in commit messages, if there's a strong feeling that that would
 be helpful then it's something I can look at doing but it also
 requires a fair bit of manual work during commit.

Can't hurt reviewer motivation.  Could it be automated?  Find replies,
extract tags.  If you want your acks to be picked up, you better make
sure your References header works, and your tags are formatted
correctly.

[...]
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] Re: KVM call minutes for June 15

2010-06-16 Thread Yoshiaki Tamura
2010/6/16 Markus Armbruster arm...@redhat.com:
 Anthony Liguori anth...@codemonkey.ws writes:

 On 06/15/2010 10:41 AM, Christoph Hellwig wrote:
 On Tue, Jun 15, 2010 at 08:18:12AM -0700, Chris Wright wrote:

 KVM/qemu patches
 - patch rate is high, documentation is low, review is low
 - patches need to include better descriptions and documentation
    - will slow down patch writers
    - will make it easier for patch reviewers

 What is the qemu patch review policy anyway?

 We don't really have a coherent policy.  Suggestions for improvement
 are always appreciated.

    There are no
 Reviewed-by: included in the actual commits,

 Reviewed-by/Ack-by's are pretty helpful for me.  In terms of including
 them in commit messages, if there's a strong feeling that that would
 be helpful then it's something I can look at doing but it also
 requires a fair bit of manual work during commit.

 Can't hurt reviewer motivation.  Could it be automated?  Find replies,
 extract tags.  If you want your acks to be picked up, you better make
 sure your References header works, and your tags are formatted
 correctly.

How about letting the submitter to include acked-by or reviewed-by
manually and repost?
It wouldn't make the maintainers busy.  Although the traffic would
increase, it would show the gratitude from submitter to the reviewer.

Thanks,

Yoshi


 [...]
 --
 To unsubscribe from this list: send the line unsubscribe kvm in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: KVM call minutes for June 15

2010-06-16 Thread Paolo Bonzini

On 06/15/2010 05:18 PM, Chris Wright wrote:

- size for each section would be useful (breaks protocol)
   - while size is possibly useful, breaks protocol


It is not necessary to break the protocol.  If you're okay with only 
having the size information when the migration data has been saved to a 
file, you can put the directory at the end of the migration data, after 
the EOF section.  Something like


   QEMU_VM_SECTION_EOF
   QEMU_VM_SECTION_DIRECTORY
  copy of the migration data, with the actual data replaced
  by a single 8-byte pointer to the beginning of the section:

  QEMU_VM_SECTION_START
  section id
  5 block
  instance id
  version id
  8-byte pointer

  QEMU_VM_SECTION_START
  section id
  3 ram
  instance id
  version id
  8-byte pointer

  ...
  QEMU_VM_SECTION_FULL
  section id
  10 cpu_common
  instance id
  version id
  8-byte pointer

  ...
  QEMU_VM_SECTION_EOF
  8-byte pointer

  QEMU_VM_SECTION_DIRECTORY
  8-byte pointer

Note that by definition the last 8 bytes will point to the beginning of 
the directory.  You can read the last 18 bytes to reduce (to almost 
zero) the possibility of a false positive.


The directory table can be built at save time and streamed after the EOF 
without causing an error if the receiver closes its connection during 
the streaming of the directory.


Paolo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] Re: KVM call minutes for June 15

2010-06-16 Thread Arnd Bergmann
On Wednesday 16 June 2010, Markus Armbruster wrote:
 Can't hurt reviewer motivation.  Could it be automated?  Find replies,
 extract tags.  If you want your acks to be picked up, you better make
 sure your References header works, and your tags are formatted
 correctly.

I think pwclient (https://patchwork.kernel.org/) can do this for you.

Arnd
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] Re: KVM call minutes for June 15

2010-06-16 Thread Blue Swirl
On Tue, Jun 15, 2010 at 4:13 PM, Anthony Liguori anth...@codemonkey.ws wrote:
 On 06/15/2010 10:41 AM, Christoph Hellwig wrote:

 On Tue, Jun 15, 2010 at 08:18:12AM -0700, Chris Wright wrote:


 KVM/qemu patches
 - patch rate is high, documentation is low, review is low
 - patches need to include better descriptions and documentation
   - will slow down patch writers
   - will make it easier for patch reviewers


 What is the qemu patch review policy anyway?

 We don't really have a coherent policy.  Suggestions for improvement are
 always appreciated.

I think a policy exists, it's just sort of 'laissez-faire'. But a
clear, written policy agreed by all wouldn't hurt.

Perhaps we should make a checklist or FAQ. I think it should be an
easy task even for someone without much QEMU background to do just
CODING_STYLE or missing SoB checks. Currently there are a lot of
commits with blatant CODING_STYLE violations. Even partial reviews for
specific issues would be helpful.


   There are no
 Reviewed-by: included in the actual commits,

 Reviewed-by/Ack-by's are pretty helpful for me.  In terms of including them
 in commit messages, if there's a strong feeling that that would be helpful
 then it's something I can look at doing but it also requires a fair bit of
 manual work during commit.

  and the requirement
 for a positive review also seem to vary a lot, up to the point that
 some commiters commit code that has never hit a public mailing list
 before.


 That really shouldn't happen and if it does, please point it out on the
 list.

This also happens due to lack of policy.

 Regards,

 Anthony Liguori

 --
 To unsubscribe from this list: send the line unsubscribe kvm in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html




--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] Re: KVM call minutes for June 15

2010-06-16 Thread Aurelien Jarno
On Tue, Jun 15, 2010 at 11:41:53AM -0400, Christoph Hellwig wrote:
 On Tue, Jun 15, 2010 at 08:18:12AM -0700, Chris Wright wrote:
  KVM/qemu patches
  - patch rate is high, documentation is low, review is low
  - patches need to include better descriptions and documentation
- will slow down patch writers
- will make it easier for patch reviewers
 
 What is the qemu patch review policy anyway?  There are no
 Reviewed-by: included in the actual commits, and the requirement
 for a positive review also seem to vary a lot, up to the point that
 some commiters commit code that has never hit a public mailing list
 before.
 

This is indeed something very useful that should be encouraged.
Depending on the patch and the persons that have reviewed/acked it, I
commit some patches only after a very quick look.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


KVM call minutes for June 15

2010-06-15 Thread Chris Wright
Page cache controls
- cache is 60% duplicated between host and guest (when not using cache=none)
- Balbir posted 2 patches to eliminate this
- boot parameter for preferred reclaim
  - not always have balloon driver
  - need a boot parameter
  - perhaps add a balloon cmd to give a hint before a more forceful request
- hard/soft quota
  - exceed hard, swap
  - exceed soft...no real penalty
  - another consideration, block device hint (has large cache in host, don't
agressively cache in guest)
- lru weighted by disk speed might make sense on bare metal
- balloon driver filter
  - balloon unmapped page cache pages first
- guest has no incentive to cooperate (give up memory and possibly performance)
  
Migration Subsections
- looks good...
- doesn't replace need to increment version numbers
- needs some documentation to make sure subtlties are clear
- size for each section would be useful (breaks protocol)
  - while size is possibly useful, breaks protocol
  - anthony wants a self-descriptive protocol if we break (like ASN.1)
  - but all moot until everything is converted to vmstate
  - need to consider specifying a new protocol

KVM/qemu patches
- patch rate is high, documentation is low, review is low
- patches need to include better descriptions and documentation
  - will slow down patch writers
  - will make it easier for patch reviewers

QMP
- spec review 
- migration events
  - introduce migration connected event
  - migration completed/done event (more contentious)
- no data, just indication that it's done
- run query migrate to get status
- 0.14 proper async
- (-rc is coming next Monday)
- important to start simple, fix as go...
- no good indication of completion in general across monitor
  - includes feedback from guest (pci remove device, shutdown, balloon, etc...)
- async is the consistent issue
- libvirt should not use QMP in 0.13
- need to declare QMP Unstable 0.13  (still need spec review)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: KVM call minutes for June 15

2010-06-15 Thread Christoph Hellwig
On Tue, Jun 15, 2010 at 08:18:12AM -0700, Chris Wright wrote:
 KVM/qemu patches
 - patch rate is high, documentation is low, review is low
 - patches need to include better descriptions and documentation
   - will slow down patch writers
   - will make it easier for patch reviewers

What is the qemu patch review policy anyway?  There are no
Reviewed-by: included in the actual commits, and the requirement
for a positive review also seem to vary a lot, up to the point that
some commiters commit code that has never hit a public mailing list
before.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: KVM call minutes for June 15

2010-06-15 Thread Anthony Liguori

On 06/15/2010 10:41 AM, Christoph Hellwig wrote:

On Tue, Jun 15, 2010 at 08:18:12AM -0700, Chris Wright wrote:
   

KVM/qemu patches
- patch rate is high, documentation is low, review is low
- patches need to include better descriptions and documentation
   - will slow down patch writers
   - will make it easier for patch reviewers
 

What is the qemu patch review policy anyway?


We don't really have a coherent policy.  Suggestions for improvement are 
always appreciated.



   There are no
Reviewed-by: included in the actual commits,


Reviewed-by/Ack-by's are pretty helpful for me.  In terms of including 
them in commit messages, if there's a strong feeling that that would be 
helpful then it's something I can look at doing but it also requires a 
fair bit of manual work during commit.



  and the requirement
for a positive review also seem to vary a lot, up to the point that
some commiters commit code that has never hit a public mailing list
before.
   


That really shouldn't happen and if it does, please point it out on the 
list.


Regards,

Anthony Liguori


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
   


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html