On Fri, Jul 15, 2011 at 12:05 PM, Kevin Wolf <kwolf@xxxxxxxxxx> wrote:
> Am 15.07.2011 12:50, schrieb Stefan Hajnoczi:
>> On Thu, Jul 14, 2011 at 7:54 PM, Adam Litke <agl@xxxxxxxxxx> wrote:
>>>
>>>
>>> On 07/14/2011 05:33 AM, Stefan Hajnoczi wrote:
>>>> On Thu, Jul 14, 2011 at 11:19 AM, Jiri Denemark <jdenemar@xxxxxxxxxx> 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@xxxxxxxxxx> 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).
Stefan
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list