qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] Xen patches - status summary


From: Ian Jackson
Subject: Re: [Qemu-devel] Xen patches - status summary
Date: Fri, 5 Sep 2008 11:33:43 +0100

Anthony Liguori writes ("Re: [Qemu-devel] Xen patches - status summary"):
> Yeah, sorry, I'm on vacation this week and my network access is much 
> worse than I expected it to be.  I'm queuing it all up and will go 
> through it this weekend when I get back.

Fair enough, that's great, thanks.  There are of course more potential
patches awaiting preparation.  I'll post another summary some time
next week, when it seems like things have changed enough.

> > I also tend to dislike os-specific stuff due to the portability issues
> > it brings.  Sometimes there are good reasons to use it nevertheless.
> 
> I haven't made up my mind yet about which approach is better.  I'll 
> commit something this weekend though.

Thanks.  Did you see Jamie's comments about the (quite alarming)
glibc 2.3.1 aio thread signal mask bug ?  If it weren't for that I
would say there's not that much to choose between our approaches but
under the circumstances I think the signal handler rather than helper
thread approach is essential.

If you are dead set on using a signalfd emulation, I could rework my
patch so that it contains an interface roughly equivalent to signalfd,
so that signalfd can easily be used where available.

> >> The QEMU_ASYNC_EVENTLOOP change is a tidying up of the NBD feature to
> >> make qemu-nbd share more commonality with qemu-img.  As discussed
> >> there are perhaps even more cleanups that could be done to improve
> >> this but as I say this change is a good start and should be applied.
> >
> > Cleanups is, well, not the correct word IMHO.  The complete block device
> > handling needs a major redesign.  That this ifdef is needed in the first
> > place is a blatant layering violation.  Also we should be able to
> > support async I/O in some form (be it libaio, threads or whatever)
> > without hacking support for it into each and every file format handler.
> 
> I have something queued up to help us get closer to this.

Err.  All that my QEMU_ASYNC_EVENTLOOP patch does is to replace
   #if !defined(QEMU_IMG) && !defined(QEMU_NBD)
with
   #ifdef QEMU_ASYNC_EVENTLOOP
which really doesn't seem controversial.  If there was some objection
to the structure of the current code, it should have been raised
before qemu-img and qemu-nbd were committed, surely ?

I think my main point in this context is that we should not make the
best be the enemy of the good.  I am carefully preparing my patches so
that they are always strictly improving, and so that any controversial
or major changes are separated out.

It is frustrating to have a trivial but clear improvement blocked by a
discussion which essentially says that it doesn't solve the entire
problem because what is really needed is a complete restructuring.

Am I supposed to hold off, and allow my trivial improvments to rot
again, for an indefinite time while awaiting the Glorious Republic to
Last a Thousand Years ?

I think the rule should generally be that a patch should be applied if
it (a) makes some improvement, no matter how minor and (b) does not
make any future desirable restructuring harder.  The fact that a
restructuring is desirable does not mean that minor improvements
should be blocked in the meantime.

Ian.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]