[vbox-dev] vgdrvIoCtl_CancelAllWaitEvents inconsistent behavior ?

[Hans de Goede]

Hi,

On 05-07-17 17:57, Michael Thayer wrote:
> Hello Hans,
> 
> 05.07.2017 15:55, Hans de Goede wrote:
> [...]
>> On 05-07-17 12:14, Michael Thayer wrote:
> [Discussion of CANCEL_ALL_WAITEVENTS use in seamless.cpp.]
> I believe your reading of the code was correct and mine not.  As per my
> last message though, see r67796[1], r67798[2] and r67802[3].

Looks good, thanks I'm still going to go for compatibility
with the old behavior though, to ensure existing guest addition
installs keep working if they end up using the in kernel
vboxguest version. Do you agree that that is the right thing to do ?

> As I mentioned, this would be the time to get more API clarifications in,
> which should help reduce your work.

Ack, if I find anything else I will let you know.

Regards,

Hans



> 
> [1] https://www.virtualbox.org/changeset/67796/vbox
> [2] https://www.virtualbox.org/changeset/67798/vbox
> [3] https://www.virtualbox.org/changeset/67802/vbox
> 
>>> Which brings us back to the question of ABI stability.  If something
>>> like this were to pop up after you had released your code, I would
>>> expect to handle it by announcing that it is deprecated and changing the
>>> ABI later, say in the next major release.  Does that seem reasonable to
>>> you, or too short?
>>
>> Since we're talking kernel ABI here now, you would need to align it
>> to the kernel cycle, not vbox major releases. I would expect a deprecation
>> like this to take at least a year, so put a note on the deprecation in:
>> linux/Documentation/ABI/obsolete/ioctl-vboxguest and add a deprecation
>> date,
>> then once that date is past you can merge a patch upstream removing the
>> backward compat stuff / enforcing new behavior and that will then get
>> merged and released in the next kernel cycle, assuming there are no
>> objections.
> Time scale objection noted, but surely the rest would be the
> responsibility for whoever is maintaining the in-kernel driver (I have a
> bit of trouble calling it the "upstream" one).  I can justify spending
> time on the in-kernel version of vboxvideo, but I don't think my
> managers would be very happy if I were to do the same for vboxguest.
> 
> [...]
>> I've finished converting the event WaitList to a standard Linux
>> wait_queue, the cleanup is not that big (yet) because I still need
>> to convert the HGCM WaitList also, after that all the WaitList
>> code can be removed which should be a nice cleanup.
>>
>> If you want to check my work see:
>> https://github.com/jwrdegoede/vboxguest/commit/dce610f4b4af6b9ad14c466cca71d0501460b2cd
>>
>>
>> Note it is probably easier to use the diff to see where the changes are
>> and then just look at the new file, as I've basically rewritten a couple
>> of functions.
> I did a single read-through (see comment above!) and it looked both good
> and equivalent to our code, as far as I could tell from that.
> 
> Regards
> Michael
>