[libvirt] [PATCHv2] blockjob: fix block-stream bandwidth race

[Jiri Denemark]

On Thu, Apr 26, 2012 at 13:08:36 -0600, Eric Blake wrote:
> With RHEL 6.2, virDomainBlockPull(dom, dev, bandwidth, 0) has a race
> with non-zero bandwidth: there is a window between the block_stream
> and block_job_set_speed monitor commands where an unlimited amount
> of data was let through, defeating the point of a throttle.
> 
> This race was first identified in commit a9d3495e, and libvirt was
> able to reduce the size of the window for that race.  In the meantime,
> the qemu developers decided to fix things properly; per this message:
> https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg03793.html
> the fix will be in qemu 1.1, and changes block-job-set-speed to use
> a different parameter name, as well as adding a new optional parameter
> to block-stream, which eliminates the race altogether.
> 
> Since our documentation already mentioned that we can refuse a non-zero
> bandwidth for some hypervisors, I think the best solution is to do
> just that for RHEL 6.2 qemu, so that the race is obvious to the user
> (anyone using stock RHEL 6.2 binaries won't have this patch, and anyone
> building their own libvirt with this patch for RHEL can also rebuild
> qemu to get the modern semantics, so it is no real loss in behavior).
> 
> Meanwhile the code must be fixed to honor actual qemu 1.1 naming.
> Rename the parameter to 'modern', since the naming difference now
> covers more than just 'async' block-job-cancel.  And while at it,
> fix an unchecked integer overflow.
> 
> * src/qemu/qemu_monitor.h (enum BLOCK_JOB_CMD): Drop unused value,
> rename enum to match conventions.
> * src/qemu/qemu_monitor.c (qemuMonitorBlockJob): Reflect enum rename.
> * src/qemu_qemu_monitor_json.h (qemuMonitorJSONBlockJob): Likewise.
> * src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockJob): Likewise,
> and support difference between RHEL 6.2 and qemu 1.1 block pull.
> * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Reject
> bandwidth during pull with too-old qemu.
> * src/libvirt.c (virDomainBlockPull, virDomainBlockRebase):
> Document this.
> ---
> 
> v2: fix integer overflow, improve variable naming
> 
>  src/libvirt.c                |    8 ++++-
>  src/qemu/qemu_driver.c       |    8 +++--
>  src/qemu/qemu_monitor.c      |   25 +++++++++++++-----
>  src/qemu/qemu_monitor.h      |   15 +++++-----
>  src/qemu/qemu_monitor_json.c |   59 +++++++++++++++++++++++------------------
>  src/qemu/qemu_monitor_json.h |    6 ++--
>  6 files changed, 72 insertions(+), 49 deletions(-)

ACK

Jirka