[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v2 02/15] blockjob: Decouple the ID from the dev
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-block] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct |
Date: |
Thu, 23 Jun 2016 14:47:06 +0200 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
On Wed 22 Jun 2016 05:49:28 PM CEST, Kevin Wolf wrote:
>> I thought adding a new 'ID' field was simpler. The device name is
>> still a device name (where it makes sense). The default ID is
>> guaranteed to be valid and guaranteed not to clash with user-defined
>> IDs. The API is (in my opinion) more clear.
>>
>> The only problems that I can think of:
>>
>> - BlockJobInfo and the events expose the 'device' field which is
>> superfluous.
>> - block-job-{pause,resume,...} can take an ID or a device name.
>
> Yes. There are two parts that I don't like about this.
>
> The first one is that we need additional code to keep track of the
> device name and to look it up.
I think this part is negligible, but ok.
> The other, more important one is that it couples block jobs more
> tightly with a BDS:
>
> * What do you with a background job that doesn't have a device name
> because it doesn't work on a BDS? Will 'device' become optional
> everywhere? But how is this less problematic for compatibility than
> returning non-device-name IDs? (To be clear, I don't think either is
> a real problem, but you can hardly dismiss one and accept the
> other.)
> * And what do you do once we allow more than one job per device? Then
> the device name isn't suitable for addressing the job any more. And
> letting the client use the device name after it started the first
> job, but not any more after it started the second one, feels wrong.
Fair enough. Unless Max, Eric or someone else has something else to add
I'll give it a try and see how it looks.
Berto
- Re: [Qemu-block] [PATCH v2 03/15] blockjob: Add block_job_get(), (continued)
- [Qemu-block] [PATCH v2 07/15] backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup', Alberto Garcia, 2016/06/22
- [Qemu-block] [PATCH v2 06/15] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror', Alberto Garcia, 2016/06/22
- [Qemu-block] [PATCH v2 08/15] stream: Add 'job-id' parameter to 'block-stream', Alberto Garcia, 2016/06/22
- [Qemu-block] [PATCH v2 05/15] blockjob: Add 'job_id' parameter to block_job_create(), Alberto Garcia, 2016/06/22
- [Qemu-block] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct, Alberto Garcia, 2016/06/22
[Qemu-block] [PATCH v2 09/15] commit: Add 'job-id' parameter to 'block-commit', Alberto Garcia, 2016/06/22
[Qemu-block] [PATCH v2 14/15] blockjob: Add 'id' parameter to 'block-job-complete', Alberto Garcia, 2016/06/22
[Qemu-block] [PATCH v2 04/15] block: Simplify find_block_job() and make it accept a job ID, Alberto Garcia, 2016/06/22
[Qemu-block] [PATCH v2 13/15] blockjob: Add 'id' parameter to 'block-job-resume', Alberto Garcia, 2016/06/22
[Qemu-block] [PATCH v2 12/15] blockjob: Add 'id' parameter to 'block-job-pause', Alberto Garcia, 2016/06/22
[Qemu-block] [PATCH v2 15/15] blockjob: Add 'id' field to 'BlockJobInfo' and all BLOCK_JOB_* events, Alberto Garcia, 2016/06/22
[Qemu-block] [PATCH v2 11/15] blockjob: Add 'id' parameter to 'block-job-cancel', Alberto Garcia, 2016/06/22