Re: [Qemu-devel] ivshmem property size should be a size, not a string

[Markus Armbruster <armbru@redhat.com>]

Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Tue, Nov 24, 2015 at 2:50 PM, Markus Armbruster <armbru@redhat.com> wrote:
>
>> Facts:
>>
>> * We have a half-initialized state that is visible to the guest.
>>
>> * To detect the doorbell, you have to wait for the device to exit this
>>   state.
>>
>> * Recognizing this state is cumbersome unless you already know you got a
>>   doorbell.
>>
>> * Most guests most of the time should take longer to boot to the point
>>   where they look at the device than the device takes to exit this
>>   state.
>>
>> This is a recipe for rare & obscure guest failure.  I doubt actual guest
>> software gets it right.  Moreover, I think it shouldn't have to get this
>> right!  It's a pointless trap for unwary guests.
>
> That's just stating that you want an easier way to deal with ivshmem
> doorbell, it doesn't mean users are unable to cope or unhappy with it.

Testing can't prove the absence of bugs.

>>>> I figure a robust guest device driver is possible, but it'll involve
>>>> dealing with traps and polling IVPosition.
>>>>
>>>> This device is simply an embarrassment.
>>>
>>> Apparently not for the people using it, or they would certainly fix it
>>> or report bugs. Yes, it could be better, but it's really not that
>>> "horrible" imho.
>>
>> We fix all kind of bugs, the ones that bite every time, and the ones
>> that can bite only when you're sufficiently unlucky.  Applies do design
>> bugs just as much as to coding bugs.
>
> Ivshmem is >5y old, not a new kid in town.

We also fix bugs regardless of age.

>> Sizable chunk of work, thank *you*!
>>
>> f7a199b ivshmem: use little-endian int64_t for the protocol
>> 660c97e ivshmem: use kvm irqfd for msi notifications
>> 0f57350 ivshmem: rename MSI eventfd_table
>> d160f3f ivshmem: remove EventfdEntry.vector
>> d9453c9 ivshmem: add hostmem backend
>> 2c04752 ivshmem: use qemu_strtosz()
>> f689d28 ivshmem: do not keep shm_fd open
>>
>>  1 file changed, 285 insertions(+), 91 deletions(-)
>
> What is this list of commits telling?

Failed attempt to show my gratitude for your work.

>>> - the new memdev property (similarly to the new use64 in c08ba66)
>>
>> I would like to take that out, yes.
>>
>> If we must have it in 2.5, mark it experimental: x-memdev.  Anybody
>> who'd want that, please speak up.
>
> This has been requested (with patches) for >2y, with several iterations:
> https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg01890.html

Thanks.

>> Post-2.5, deprecate the existing device model for something more sane.
>> I'm thinking of a pair of device models, one with and one without the
>> doorbell.  Make sure they have a clean guest ABI, a clean set of
>> properties, and a reasonable split between frontend and backend.  If we
>> kept x-memdev, drop it from the deprecated device.
>>
>>> Imho it's not such a big deal to have a compatibility interface with
>>> 2.5 in the future, and I would rather keep the consistant behaviour
>>> for the error return case.
>>
>> The fewer compatibility interfaces we maintain, and the simpler they
>> are, the better.  I don't see why we must complicate this one before we
>> deprecate it when we can it the other way round.
>
> I am just saying I am okay with this extra property.

Noted.