[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the dev
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct |
Date: |
Fri, 1 Jul 2016 14:48:06 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 |
On 06/30/2016 09:03 AM, Alberto Garcia wrote:
> On Wed 29 Jun 2016 07:20:55 PM CEST, Max Reitz 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.
>>
>> Sorry for the late response, but then again I don't actually have an
>> opinion either way.
>>
>> The thing I feel most strongly about is the issue of storing a general
>> ID in a field named "device". However, as Kevin hinted at this
>> becoming irrelevant with John's work on decoupling block jobs from the
>> block layer.
>
> I actually forgot to Cc him, I'm doing it now.
>
> The idea is that I don't want to add anything now that is going to cause
> headaches later. Adding a new 'id' field to block jobs and keeping
> 'device' feels more natural to me, but reusing the 'device' field and
> allowing any ID set by the user requires less changes both to the code
> and the API.
>
> Berto
>
Reviewing everything now, sorry for being MIA, and thank you for keeping
me in the loop.
--js
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct,
John Snow <=