KVM call minutes for June 15

KVM call minutes for June 15

[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)

[Qemu-devel] KVM call minutes for June 15

[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)

Re: KVM call minutes for June 15

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

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

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

Re: KVM call minutes for June 15

[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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>    

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

[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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>    

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

[Markus Armbruster]

Anthony Liguori <anthony@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.

[...]

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

[Yoshiaki Tamura]

2010/6/16 Markus Armbruster <armbru@redhat.com>:
> Anthony Liguori <anthony@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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

[Yoshiaki Tamura]

2010/6/16 Markus Armbruster <armbru@redhat.com>:
> Anthony Liguori <anthony@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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

[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

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

[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

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

[Blue Swirl]

On Tue, Jun 15, 2010 at 4:13 PM, Anthony Liguori <anthony@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 majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
>
>

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

[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
aurelien@aurel32.net                 http://www.aurel32.net

Re: KVM call minutes for June 15

[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

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

[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