[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Fri, Jul 15, 2011 at 12:07 PM, Stefan Hajnoczi <stefanha gmail com> wrote:
> On Fri, Jul 15, 2011 at 12:05 PM, Kevin Wolf <kwolf redhat com> wrote:
>> Am 15.07.2011 12:50, schrieb Stefan Hajnoczi:
>>> On Thu, Jul 14, 2011 at 7:54 PM, Adam Litke <agl us ibm com> wrote:
>>>>
>>>>
>>>> On 07/14/2011 05:33 AM, Stefan Hajnoczi wrote:
>>>>> On Thu, Jul 14, 2011 at 11:19 AM, Jiri Denemark <jdenemar redhat com> wrote:
>>>>>> On Thu, Jul 14, 2011 at 10:58:31 +0100, Stefan Hajnoczi wrote:
>>>>>>> On Thu, Jul 14, 2011 at 10:21 AM, Jiri Denemark <jdenemar redhat com> wrote:
>>>>>>>> On Wed, Jul 13, 2011 at 15:46:30 -0500, Adam Litke wrote:
>>>>>>>> ...
>>>>>>>>> /*
>>>>>>>>>  * BlockPull API
>>>>>>>>>  */
>>>>>>>>>
>>>>>>>>> /* An iterator for initiating and monitoring block pull operations */
>>>>>>>>> typedef unsigned long long virDomainBlockPullCursor;
>>>>>>>>>
>>>>>>>>> typedef struct _virDomainBlockPullInfo virDomainBlockPullInfo;
>>>>>>>>> struct _virDomainBlockPullInfo {
>>>>>>>>>     /*
>>>>>>>>>      * The following fields provide an indication of block pull progress.   cur
>>>>>>>>>      * indicates the current position and will be between 0 and @end.   end is
>>>>>>>>>      * the final cursor position for this operation and represents completion.
>>>>>>>>>      * To approximate progress, divide @cur by @end.
>>>>>>>>>      */
>>>>>>>>>     virDomainBlockPullCursor cur;
>>>>>>>>>     virDomainBlockPullCursor end;
>>>>>>>>> };
>>>>>>>>> typedef virDomainBlockPullInfo *virDomainBlockPullInfoPtr;
>>>>>>>> ...
>>>>>>>>> /**
>>>>>>>>>  * virDomainBlockPullAbort:
>>>>>>>>>  * @dom: pointer to domain object
>>>>>>>>>  * @path: fully-qualified filename of disk
>>>>>>>>>  * @flags: currently unused, for future extension
>>>>>>>>>  *
>>>>>>>>>  * Cancel a pull operation previously started by virDomainBlockPullAll().
>>>>>>>>>  *
>>>>>>>>>  * Returns -1 in case of failure, 0 when successful.
>>>>>>>>>  */
>>>>>>>>> int                 virDomainBlockPullAbort(virDomainPtr dom,
>>>>>>>>>                                             const char *path,
>>>>>>>>>                                             unsigned int flags);
>>>>>>>>>
>>>>>>>>> /**
>>>>>>>>>  * virDomainGetBlockPullInfo:
>>>>>>>>>  * @dom: pointer to domain object
>>>>>>>>>  * @path: fully-qualified filename of disk
>>>>>>>>>  * @info: pointer to a virDomainBlockPullInfo structure
>>>>>>>>>  * @flags: currently unused, for future extension
>>>>>>>>>  *
>>>>>>>>>  * Request progress information on a block pull operation that has been started
>>>>>>>>>  * with virDomainBlockPull().  If an operation is active for the given
>>>>>>>>>  * parameters, @info will be updated with the current progress.
>>>>>>>>>  *
>>>>>>>>>  * Returns -1 in case of failure, 0 when successful.
>>>>>>>>>  */
>>>>>>>>> int                 virDomainGetBlockPullInfo(virDomainPtr dom,
>>>>>>>>>                                               const char *path,
>>>>>>>>>                                               virDomainBlockPullInfoPtr info,
>>>>>>>>>                                               unsigned int flags);
>>>>>>>>>
>>>>>>>>> /**
>>>>>>>>>  * virConnectDomainEventBlockPullStatus:
>>>>>>>>>  *
>>>>>>>>>  * The final status of a virDomainBlockPull() operation
>>>>>>>>>  */
>>>>>>>>> typedef enum {
>>>>>>>>>     VIR_DOMAIN_BLOCK_PULL_COMPLETED = 0,
>>>>>>>>>     VIR_DOMAIN_BLOCK_PULL_FAILED = 1,
>>>>>>>>> } virConnectDomainEventBlockPullStatus;
>>>>>>>>>
>>>>>>>>> /**
>>>>>>>>>  * virConnectDomainEventBlockPullCallback:
>>>>>>>>>  * @conn: connection object
>>>>>>>>>  * @dom: domain on which the event occurred
>>>>>>>>>  * @path: fully-qualified filename of the affected disk
>>>>>>>>>  * @status: final status of the operation (virConnectDomainEventBlockPullStatus
>>>>>>>>>  *
>>>>>>>>>  * The callback signature to use when registering for an event of type
>>>>>>>>>  * VIR_DOMAIN_EVENT_ID_BLOCK_PULL with virConnectDomainEventRegisterAny()
>>>>>>>>>  */
>>>>>>>>> typedef void (*virConnectDomainEventBlockPullCallback)(virConnectPtr conn,
>>>>>>>>>                                                        virDomainPtr dom,
>>>>>>>>>                                                        const char *path,
>>>>>>>>>                                                        int status,
>>>>>>>>>                                                        void *opaque);
>>>>>>>>
>>>>>>>> Could these managing functions and event callback become general and usable by
>>>>>>>> other block operations such as block copy?
>>>>>>>
>>>>>>> Hi Jiri,
>>>>>>> Live block copy will certainly be possible using the streaming API.
>>>>>>> Before you start streaming you need to use the snapshot_blkdev command
>>>>>>> to create the destination file with the source file as its backing
>>>>>>> image.  You can then use the streaming API to copy data from the
>>>>>>> source file into the destination file.  On completion the source file
>>>>>>> is no longer needed.
>>>>>>
>>>>>> Well, I'm not talking about using the same API for block copy or implementing
>>>>>> block copy internally as block streaming. I'm talking about making GetInfo,
>>>>>> Abort and event callback general to be usable not only for block streaming or
>>>>>> block copy but also for other possible block operations in the future.
>>>>>>
>>>>>> The reason is that starting a block operation (streaming, copy, whatever) may
>>>>>> need different parameters so they should be different APIs. But once the
>>>>>> operation is started, we just need to know how far it got and we need the
>>>>>> ability to abort the job. So this can share the same APIs for all operations.
>>>>>> It doesn't make sense to require any block operation to provide their own set
>>>>>> of managing APIs.
>>>>>>
>>>>>> It's analogous to virDomainSave, virDomainMigrate, virDomainCoreDump. They are
>>>>>> all different jobs but all of them can be monitored and aborted using
>>>>>> virDomainGetJobInfo and virDomainAbortJob.
>>>>>
>>>>> I understand.  I can't comment on libvirt API specifics but yes,
>>>>> there's a similarity here and a chance we'll have other operations in
>>>>> the future too.
>>>>>
>>>>> I'm thinking things like compacting images, compressing images, etc.
>>>>
>>>> From a libvirt API perspective, I would only want to merge the abort and
>>>> info commands if the underlying qemu monitor commands are also merged.
>>>> Otherwise, we'll need to maintain state on which types of jobs are
>>>> active on which devices and the added complexity erases any potential
>>>> benefits IMO.
>>>>
>>>> Stefan, any chance that block operation info and cancellation could be
>>>> unified at the qemu level?
>>>
>>> The commands would become "block_job_cancel" and "info block-job".
>>>
>>> block_stream would start a job, which basically means:
>>> BlockJob {
>>>     BlockDriverState *bs; /* the block device */
>>>     BlockJobType type; /* BLOCK_JOB_STREAMING */
>>>     int64_t offset;    /* progress info */
>>>     int64_t end;
>>> };
>>>
>>> We probably want to generalize the offset/end pair to have no units.
>>> They are simply used to indicate progress information.  When the job
>>> completes offset == end.
>>>
>>> The BLOCK_STREAM_COMPLETED event also needs to be generalized to
>>> BLOCK_JOB_COMPLETED.
>>>
>>> This approach is reasonable although I do worry that we don't
>>> understand the requirements for future block device background jobs.
>>> For example, jobs that perform an operation on two block devices
>>> instead of just one.  With QMP we are extensible and can add fields
>>> for specific job types, so hopefully we can get by without finding the
>>> API inadequate for future block jobs.
>>>
>>> Kevin: What do you think about providing a job interface instead of
>>> tying the cancel and info operations specifically to block_stream?
>>
>> I guess it's a reasonable thing to do. We shouldn't overengineer here
>> and try to solve all problems in qemu at once, but as long as we're
>> basically talking about renaming what our current streaming/block copy
>> concept needs anyway, I don't see a problem.
>
> I would leave block_stream as-is because the job start function is
> where most of the variability comes in.  I think it's not worth doing
> a generic block_job_start function where you have to say
> type=streaming.
>
> /me is off to update the image streaming API wiki page (and have lunch).

The wiki is updated with block_job APIs and block_stream_set_speed:
http://wiki.qemu.org/Features/LiveBlockMigration/ImageStreamingAPI

Stefan


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]