[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 0/4] block: add optional 'speed' parameter to bl
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH 0/4] block: add optional 'speed' parameter to block-stream |
Date: |
Mon, 23 Apr 2012 15:04:36 -0300 |
On Mon, 23 Apr 2012 11:33:27 -0600
Eric Blake <address@hidden> wrote:
> On 04/23/2012 09:39 AM, Stefan Hajnoczi wrote:
> > This series is based on Luiz' latest QMP pull request at
> > git://repo.or.cz/qemu/qmp-unstable.git queue/qmp
> > (d980956d5bfa9f4fd56c00a0b168c3c0f67bc0d2). This dependency is necessary
> > because this series conflicts with the block_stream -> block-stream rename.
> >
> > Eric Blake raised concerns about the inability to start block jobs with a
> > speed
> > limit. Current the user needs to follow up the block-stream command with
> > block-job-set-speed. There is a window of time while the new block job is
> > running but block-job-set-speed has not been processed yet.
> >
> > This series adds an optional 'speed' parameter to block-stream so streaming
> > can
> > be started with a speed limit that takes effect immediately.
> >
> > I considered several other approaches, including adding a
> > default_block_job_speed field to BlockDriverState but ultimately the
> > cleanest
> > solution is to pass in a speed parameter on job creation. This way we do
> > not
> > change semantics of existing commands, we only add an optional parameter.
> > We
> > also do not need to add state to BlockDriverState, which is already huge and
> > messy.
>
> I like the approach of adding a new optional parameter; that is a
> workable solution for libvirt (that is, libvirt can assume that if the
> spelling 'block-stream' is available, then so is the optional 'speed'
> parameter, if this patch is included in time for qemu 1.1).
The optional parameter is fine for me too, but we recently had a
discussion where it was decided that we should add new commands instead
of extending existing ones.
Anthony, I believe we're already taking that position, right?