[Qemu-devel] Is ivshmem's test for unix domain client socket valid?

[Qemu-devel] Is ivshmem's test for unix domain client socket valid?

[Markus Armbruster]

Looks rather fishy:

        if (strncmp(s->server_chr->filename, "unix:", 5)) {
            error_setg(errp, "chardev is not a unix client socket");
            return;
        }

Paolo, is this reliable?  Is it the proper way to check?

Re: [Qemu-devel] Is ivshmem's test for unix domain client socket valid?

[Paolo Bonzini]

On 20/11/2015 18:56, Markus Armbruster wrote:
> Looks rather fishy:
> 
>         if (strncmp(s->server_chr->filename, "unix:", 5)) {
>             error_setg(errp, "chardev is not a unix client socket");
>             return;
>         }
> 
> Paolo, is this reliable?

Yes, though by total chance (which is already a curious definition of
reliability).  If you create "-chardev file,path=unix:foo",
chr->filename ends up with "file" because only pty and socket chardevs
set chr->filename.  Everything else does not set it, and the field is
thus set to the default---the backend name.

> Is it the proper way to check?

It's the only one, which makes it the most proper too.  Sarcasm is
intentional.

Paolo

[Qemu-devel] Is ivshmem's use of qemu_chr_open_eventfd() kosher? (was: Is ivshmem's test for unix domain client socket valid?)

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 20/11/2015 18:56, Markus Armbruster wrote:
>> Looks rather fishy:
>> 
>>         if (strncmp(s->server_chr->filename, "unix:", 5)) {
>>             error_setg(errp, "chardev is not a unix client socket");
>>             return;
>>         }
>> 
>> Paolo, is this reliable?
>
> Yes, though by total chance (which is already a curious definition of
> reliability).  If you create "-chardev file,path=unix:foo",
> chr->filename ends up with "file" because only pty and socket chardevs
> set chr->filename.  Everything else does not set it, and the field is
> thus set to the default---the backend name.
>
>> Is it the proper way to check?
>
> It's the only one, which makes it the most proper too.  Sarcasm is
> intentional.

FIXME added in my local ivshmem for-2.5 branch.

Next question: ivshmem creates a character backend in
create_eventfd_chr_device() like this

        chr = qemu_chr_open_eventfd(eventfd);

on behalf of setup_interrupt()

    static void setup_interrupt(IVShmemState *s, int vector)
    {
        EventNotifier *n = &s->peers[s->vm_id].eventfds[vector];
        bool with_irqfd = kvm_msi_via_irqfd_enabled() &&
            ivshmem_has_feature(s, IVSHMEM_MSI);
    [...]
        if (!with_irqfd) {
            IVSHMEM_DPRINTF("with eventfd");
            s->eventfd_chr[vector] = create_eventfd_chr_device(s, n, vector);
        } else [...]

Device models (a.k.a. frontends) are normally not supposed to create
backends by themselves, because it blurs the separation between frontend
and backend, and commonly denies the user the opportunity to fully
configure the backend.

Most other devices that do, as far as I can see, create null chardevs
when the user didn't specify one.  That's fine.  Exceptions:

* xen_console.c's con_init()

  Part of Xen's bigger "get configuration from XenStore" exception.

* dev-serial.c's usb_serial_init() and usb_braille_init()

  Legacy -usbdevice.

None of them qualifies as precedence.

Is ivshmem's implicit creation of a backend appropriate?

I wish I had an easy way to find device models doing something no other
does.

Re: [Qemu-devel] Is ivshmem's use of qemu_chr_open_eventfd() kosher?

[Paolo Bonzini]

On 24/11/2015 13:26, Markus Armbruster wrote:
> Device models (a.k.a. frontends) are normally not supposed to create
> backends by themselves, because it blurs the separation between frontend
> and backend, and commonly denies the user the opportunity to fully
> configure the backend.

Right, but here the fd comes from the ivshmem server.  It's just using
the chardev API internally.  It could be changed with little effort to
use a GSource or qemu_set_fd_handler.

Paolo

Re: [Qemu-devel] Is ivshmem's use of qemu_chr_open_eventfd() kosher?

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 24/11/2015 13:26, Markus Armbruster wrote:
>> Device models (a.k.a. frontends) are normally not supposed to create
>> backends by themselves, because it blurs the separation between frontend
>> and backend, and commonly denies the user the opportunity to fully
>> configure the backend.
>
> Right, but here the fd comes from the ivshmem server.  It's just using
> the chardev API internally.  It could be changed with little effort to
> use a GSource or qemu_set_fd_handler.

Thanks!