[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu
Gerd Hoffmann, le Thu 07 Aug 2008 09:33:06 +0200, a écrit : > Samuel Thibault wrote: > > Samuel Thibault, le Wed 06 Aug 2008 15:25:26 +0100, a écrit : > >> Pushing the cleaning changes to Xen first can be done and would entail > >> much easier to tackle breakage, and the merge back from qemu would then > >> be trivial, why not doing so? > > > > You didn't answer that part. Really, my only concern is about having > > things tested. Isn't it possible for instance to just merge the backend > > core (and console/xenfb updates) in Ian's tree first for a start? > > http://kraxel.fedorapeople.org/patches/qemu-xen/ Ok, thanks, that's already better. I'd still say that "delete a file then add another one" is far from convenient both from a point of view of proof-reading the patches and repository history, but I guess we can manage that with a renaming step first. Any reason for the renames, though? (they tend to bother developpers who have to change their habits, so we can not do that without a reason) - Why dashes instead of underscores? - In Xen, we call xen-machine.c xen_machine_pv.c because there is also a xen_machine_fv.c. Can't the xen_machine_pv.c name be fine for qemu upstream too? Or xen can keep its own xen_machine_{pv,fv}.c and your xen-machine.c goes upstream, I don't think that would be a problem. - I guess a xenfb -> xen[-_]{fb,framebuffer} rename is indeed more coherent, Markus, do you prefer just "fb" or the longer "framebuffer"? Misc stuff: - I guess the sys-queue.h file should be merged and used in qemu first? - xen_domid is re-declared in xen-backend.h - The BUFSIZE macro in xen-backend.h is a bit unfortunate, but still better than what we have currently. Maybe it should be renamed with a - XEN_ prefix to avoid any clash? - container_of would probably be useful in other parts of qemu, I guess it could be merged in qemu first? - nice work on moving stuff from console and fb to the backend, console is easier to read now :) > I also largely left vl.c as-is, so xend shouldn't need any changes. The > -domid switch sets an additional (redundant) variable, to keep the > amount of changes as small as possible for now. Also -name and > -domain-name are aliased, both set qemu_name and domain_name. That's a good thing :) > The framebuffer driver probably has some performance regressions. > Fixing those depends on the display patches being pushed upstream. It would have been easier to review if you had provided a delta patch, not a delete/add patch </grin>. That's actually the kind of things I was afraid of and that would have been harder to spot when just pulling your work from qemu upstream. In your xen patch, since idle and GUI_REFRESH_INTERVAL are there, you can just use them. We'll push them to qemu. We'll manage the _shared stuff (and push it eventually). I'll let Markus comment on the rest (again, thanks for moving the xenbus stuff to the backend part). There is one change that is not backend changes or just moving code around: you are queuing rectangle updates, why? (I'm not arguing, just wondering the kind of optimization that can be). > > Then you can push your code to qemu, > > I guess that could be fine, as you said xen will not need to use e.g. > > the block and net backends. > > blk and net backends are not there (yet). But they should be a nop for > xen anyway Indeed, I'd say don't bother to port those. > The block backend should be a good replacement for blktap though and > maybe can save you the effort of porting the blktap kernel driver to > the pv_ops kernel. Well, for better performance an in-kernel blktap would still be useful. Samuel _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |