Re: [Xen-devel] Re: [PATCH 0/2] PV framebuffer

[Markus Armbruster <armbru@xxxxxxxxxx>]

Steven Smith <sos22-xen@xxxxxxxxxxxxx> writes:

>> Next iteration.  If there's anything left in the way of getting this
>> merged, please let me know right away.
>> 
>> Changes since last time:
>> 
>> * Leave xen console defaulting to /dev/tty1.
>> 
>> * Start backend automatically.
> I'm not entirely happy with the way this is done; I'd much rather the
> device controller started the backend than the image controller.  I
> can probably fix this up more easily than you.

No problem.

>> * Fix kernel thread to call try_to_freeze().
>> 
>> * Recreate the image sub-object in XendDomainInfo.py on resume.
> Why did you do this?  I can't see that it has anything to do with the
> pvfb work, and everything seems to still work when I get rid of this
> part of the patch.

Without it, the framebuffer doesn't survive a save/restore.

There may well be a better way to restart the backend on restore.

>> To activate the framebuffer, you have to put the following lines into
>> /etc/xen/DOMNAME:
>> 
>>     vfb = 'yes'
>>     vkbd = 'yes'
>> 
>> and either
>> 
>>     sdl = 1
>> 
>> or
>> 
>>     vnc = 1
> I'm not entirely happy about this configuration syntax:
>
> -- It's redundant: you can't specify a vfb without a vkbd or
>    vice-versa, or the domain won't start, so we might as well get rid
>    of the vkbd="yes" business

Absolutely.  It was just the simplest conceivable way to get the thing
to work.

> -- It doesn't give any way of specifying parameters to the backend
> -- The configuration for a single device ends up split into several
>    different items.  It also depends on stuff outside the usual
>    (device) sections in the SXP, which is kind of nasty.
> -- It potentially conflicts with the way emulated vga is specified in
>    hvm guests.  e.g. you can't specify that emulated vga should be
>    exported over vnc while the pv framebuffer should be sdl.  Whether
>    you'd ever want to do this is another matter, but it'd be nice if
>    we didn't rule it out for reasons as trivial as a bad configuration
>    file design.
> -- It won't extend to supporting multiple pvfbs
> -- It doesn't look much like the syntax for configuring other devices.
>
> I'd prefer something more like this:
>
> vfbs = [ 'type=sdl,listen=127.0.0.1' ]
>
> Again, I can probably fix this up more easily than you, if you don't
> object.

Not at all.

>> One last thing to consider: I'm not entirely happy with source file
>> names and locations.  It's now or never for renames.  Peeves:
>> 
>> * Frontend source usually lives in drivers/xen/<WHAT>front.  xenfb and
>>   xenkbd are exceptions.  Rename to fbfront and kbdfront?  Same for
>>   the .c files.  This would also fix the minor annoyance of having the
>>   same file name in frontend and backend.
>> 
>> * Given how closely related xenfb and xenkbd are, I'm not fond of
>>   having them in separate directories.  Move kbd into the fb
>>   directory?
> If it were my patch, I'd put everything in drivers/xen/fbfront, but it
> doesn't really make a great deal of difference.
>
> Apart from that, I'm happy with this.  If you send me a patch which
> applies cleanly to xen-unstable and is signed-off-by the right people,
> I'll check it in and fix the things I mentioned myself.

Deal!

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel