Re: [libvirt] RFC New virDomainBlockPull API family to libvirt
On Fri, Jul 15, 2011 at 3:09 PM, Adam Litke <agl(a)us.ibm.com> wrote:
On 07/15/2011 05:39 AM, Stefan Hajnoczi wrote:
> On Thu, Jul 14, 2011 at 7:47 PM, Adam Litke <agl(a)us.ibm.com> wrote:
>> On 07/13/2011 08:04 PM, Daniel Veillard wrote:
>>> On Wed, Jul 13, 2011 at 03:46:30PM -0500, Adam Litke wrote:
>>>> Unfortunately, after committing the blockPull API to libvirt, the qemu
>>>> community decided to change the API somewhat and we needed to revert the
>>>> libvirt implementation. Now that the qemu API is settling out again, I
would
>>>> like to propose an updated libvirt public API which has required only a
minor
>>>> set of changes to sync back up to the qemu API.
>>>>
>>>> Summary of changes:
>>>> - Qemu dropped incremental streaming so remove libvirt incremental
>>>> BlockPull() API
>>>> - Rename virDomainBlockPullAll() to virDomainBlockPull()
>>>> - Changes required to qemu monitor handlers for changed command names
>>>
>>>
>>> Okay.
<snip>
>>> On the other hand I suspect that we are missing the mechanism to
>>> control the rate of the transfer in the new API, which is something
>>> which could be implemented in the old incremental mechanism, but not
>>> anymore. So I wonder if the virDomainBlockPull() shouldn't get an
>>> "unsigned long bandwidth" (to stay coherent with migration APIs)
>>> before the flags. I don't know if the support is ready in QEmu but
>>> I suspect that will be an important point, so if not present will
>>> be added :-)
>>
>> Hopefully Stefan can comment on any throttling mechanisms that might be
>> added to the qemu implementation. It would be nice to have this feature
>> built into the API to match the migration APIs, but if that is not
>> feasible then we could always add it later.
>
> If we follow the live migration API then a block_stream_set_speed
> command would be used. libvirt would issue it as a separate command
> immediately after starting the streaming operation. There is the
> window of time after streaming has started but before
> block_stream_set_speed has been called where no throttling takes
> place, but I don't think this is a practical problem.
>
> It also opens the possibility of allowing the user to change the rate
> limit value while the block streaming operation is active.
In light of our decision to provide a generic BlockJobAbort/BlockJobInfo
interface, should SetSpeed be generic too?
virDomainBlockJobSetSpeed() or virDomainBlockPullSetSpeed() ?
The block_stream_set_speed semantics allow the command to be used when
no streaming operation is active. This can be used to set the speed
without a window of time after creation and before set_speed is
called.
If we switch to block_job_set_speed then it is cleaner to restrict the
command to work on active jobs only. This is because there is only
one "speed" variable kept but there might be multiple job types
(streaming, compaction, etc) which would need different settings.
What do you think about this?
block_job_set_speed
-------------------
Set maximum speed for a background block operation.
This is a per-block device command that can only be issued
when there is an active block job.
Throttling can be disabled by setting the speed to 0.
Arguments:
- device: device name (json-string)
- value: maximum speed, in bytes per second (json-int)
Errors:
NotSupported: job type does not support speed setting
Example:
-> { "execute": "block_job_set_speed",
"arguments": { "device": "virtio0", "value":
1024 } }
Stefan