qemu-devel
[Top][All Lists]
Advanced

[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: Alberto Garcia
Subject: Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct
Date: Thu, 30 Jun 2016 15:03:47 +0200
User-agent: Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu)

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]