Re: [libvirt] RFC New virDomainBlockPull API family to libvirt

Am 20.07.2011 15:12, schrieb Stefan Hajnoczi: > On Wed, Jul 20, 2011 at 2:00 PM, Stefan Hajnoczi <stefanha(a)gmail.com&gt; wrote: >> On Mon, Jul 18, 2011 at 9:10 PM, Adam Litke <agl(a)us.ibm.com&gt; wrote: >>> On 07/18/2011 09:35 AM, Stefan Hajnoczi wrote: >>>>>>>>   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? >>> >>> I think the block_job_set_speed semantics seem reasonable to me. What is >>> the method for querying the current speed setting?  I would suggest >>> extending the query-block-job command to include this information.  In >>> this case, I would extend virDomainBlockJobInfo to contain: >>> >>>        /* The maximum allowable bandwidth for this job (MB/s) */ >>>        unsigned long bandwidth; >> >> Working QEMU code is pushed to: >> http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/stream-command >> >> I am updating the QEMU wiki page with the latest changes. > > v4 posted: > http://wiki.qemu.org/Features/LiveBlockMigration/ImageStreamingAPI > > Adam: block_job_set_speed fully updated. > > Kevin: we talked about error values earlier.  Internally we really > only have an errno value when streaming fails.  I have converted this > to a human-readable string. If the human can read English. :-) And even though I do, I really don't like messages like I got from the Fedora updater in earlier versions ("You have 42 Aktualisierungen"), so plain strings aren't going to work well. This is true even for localised strerror output, as the client may use a different language than the server. I think this has been discussed multiple time in the past (we wanted to somehow expose something useful on I/O errors).