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