Re: [Qemu-devel] [PATCH 1/3] iothread: provide helpers for internal use

[Daniel P. Berrange]

On Fri, Sep 22, 2017 at 03:35:05PM +0100, Stefan Hajnoczi wrote:
> On Fri, Sep 22, 2017 at 11:28:08AM +0100, Daniel P. Berrange wrote:
> > On Fri, Sep 22, 2017 at 12:26:16PM +0200, Paolo Bonzini wrote:
> > > On 22/09/2017 12:20, Daniel P. Berrange wrote:
> > > > On Fri, Sep 22, 2017 at 12:18:44PM +0200, Paolo Bonzini wrote:
> > > >> On 22/09/2017 12:16, Stefan Hajnoczi wrote:
> > > >>> I suggest adding internal IOThreads alongside user-created IOThreads
> > > >>> instead of hiding them.  IOThread also needs a bool user_created field
> > > >>> and a UserCreatableClass->can_be_deleted() function:
> > > >>>
> > > >>>   static bool iothread_can_be_deleted(UserCreatable *uc)
> > > >>>   {
> > > >>>       return IOTHREAD(uc)->user_created;
> > > >>>   }
> > > >>>
> > > >>> This way users cannot delete internal IOThreads.
> > > >>>
> > > >>> But how should object ids be handled?  In theory existing -object
> > > >>> iothread,id=<id> users could use any name.  How can QEMU generate ids
> > > >>> for internal IOThreads without conflicting with existing users's ids?
> > > >>
> > > >> I would add an 'internal' boolean to query-iothreads' response and a 
> > > >> new
> > > >> 'show-internal' boolean to the command.  This way, applications that
> > > >> request internal iothreads would know that the "primary key" is
> > > >> (internal, id) rather than just the id.
> > > > 
> > > > What is the app going to do with iothreads if it sees "internal" flag
> > > > set ? They have no way of knowing what part of QEMU internally is using
> > > > this iothread, so I don't see that they can do anything intelligent
> > > > once they find out they exist.
> > > 
> > > The application could apply them default settings for scheduler policy
> > > or CPU affinity.
> > > 
> > > Unlike the main or the I/O thread, the monitor thread doesn't interrupt
> > > the CPU, so it need not run at SCHED_FIFO even in real-time settings.
> > 
> > There's no way for the application to know that. ALl it sees is that there
> > is an extra unexpected IOthread created internally by QEMU. There's no way
> > to decide whether it nedes SCHED_FIFO or not as we've no clue what it is
> > being used for.
> 
> For observability (troubleshooting, debugging, etc) I would like all
> IOThreads to be visible.  That makes it much easier to troubleshoot in a
> unified way rather than having to get at internal IOThreads through
> different means (e.g. gdb).
> 
> If QEMU uses a well-known ID for the internal monitor IOThread then
> libvirt could also interact with it (e.g. CPU affinity).

I don't think we should abuse ID values by turning them into stable
ABI - currently they are opaque tokens whose value has no special
meaning and make change at will over time.

If we need to be able to report the IOthread for the monitor, then it
would have to be done via a formal API, but that turns the use of
IOThreads for the monitor into a feature we have to keep indefinitely,
even if we wanted to re-implement it differently later.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|