[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
> 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. Fine with me, I can't send patches with renames though ... > 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) Get consistent naming (all xen stuff in hw/ is prefixed with xen-). > - Why dashes instead of underscores? (1) It looks more pleasent to my eyes. (2) It is easier to type (no shift needed) on both us and german keyboards. (3) The files in the qemu source tree don't have a consistent style in respect to '-' vs. '_', so I had no reason to not use my personal preference ;) > - 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? Dunno whenever there ever will be a xenfv machine type upstream as most likely the normal 'pc' machine type will get a fancy interface to switch between emulation, kqemu and kvm. And I think the best option for xen hvm would be to hook in there too. That is a long-term thing though, xen_machine_fv.c will probably stay for quite a while at in the xen tree, so maybe 'pv' in the name helps avoiding confusion. I'd prefer to stay with the xen-machine.c though. > 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. We have two files defining xenpv_machine then. I don't think it is a good idea. > Misc stuff: > - I guess the sys-queue.h file should be merged and used in qemu first? Rebase. Then it will be there, and you can skip the patch ;) > - xen_domid is re-declared in xen-backend.h Oops. Fixed. > - 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? xen-backend.h is supposed to be included by xen-related code only (it barfs when /usr/include/xen isn't there), so it shouldn't clash in theory. Well, I can prefix it nevertheless, better safe than sorry. > - container_of would probably be useful in other parts of qemu, I guess > it could be merged in qemu first? Has qemu a nice place for that kind of helper macros? > 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'd prefer to do it the other way around (push depending changes upstream, then adapt xen-framebuffer.c). I want xen-framebuffer.c look the same in xen and upstream. > 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). Thats actually a bugfix. Doing screen updates outside the ->update callback isn't safe. And as we must queuing up update rectangles anyway to fix that it also tries to optimize stuff a bit and avoid unneeded bitblits. Right now that catches only frequent fullscreen updates (such as a scrolling textconsole) and doesn't copy stuff multiple times in case the update events from the guest are more frequent than update callback calls from qemu. Could be improved to analyze the rectangles in more detail (for example: figure a update is a superset of another one, so one can be dropped). >> 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. Don't confuse blkback and blktap. I don't want to replace the in-kernel blkback driver. blktap isn't a in-kernel driver though. It is just an interface between the hypervisor and a userspace application, specialized for block devices. My backend driver does pretty much the same, but uses the generic gntdev interface instead of blktap. I can't see any reason why it shouldn't match blktap performance-wise. I have no benchmarks numbers though. cheers, Gerd -- http://kraxel.fedorapeople.org/xenner/ _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |