Re: [Xen-devel] [PATCH v4] add support for libvirt-like channels

[Dave Scott <Dave.Scott@xxxxxxxxxx>]

Hi,

On 10 Sep 2014, at 14:07, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:

> On Mon, 2014-07-28 at 15:22 +0100, David Vrabel wrote:
>> On 22/07/14 16:05, David Scott wrote:
>>> 
>>> Note: I've seen a problem with some Linux console frontends which attempt
>>> to overwrite the read-only key 'type' (= xenconsoled|ioemu) in the frontend
>>> directory. [ this key is already present, it's not added by these patches ]
>>> Since 'type' refers to the toolstack's choice of implementation service
>>> (which is none of the guest's business) I think the read/only permissions
>>> are right. The location of the key is not ideal (it should be in the
>>> backend directory IMHO) but this is the case with several other keys located
>>> in the frontend such as 'limit' and 'tty'.
> 
> I think this probably arose because the first PV console is not (or at
> least historically wasn't) a normal front/back pair so I think it may
> only have had a f.e. directory. It's likely that this then carried over
> into the support for multiple console channels which do follow the
> front/back model.

Aha, that explains a lot.

> 
>> I think this is a Linux frontend
>>> bug which should be fixed there.
> 
> Yeah, irrespective of the above it was never right for the frontend to
> try and write this node AFAICT.
> 
>> These patches work with Mirage VMs and
>>> with Linux if I workaround the permissions by giving the guest read/write
>>> to the 'type' key. 
>> 
>> Which Linux frontends?
> 
> Did this get answered/sorted (if so then I'm afraid I've lost that
> subthread).

Yes, Stefano and David removed the write in the Linux frontend:

https://lkml.org/lkml/2014/8/11/219

I’m not sure if this has been merged yet.

> Any idea why this only happens with these patches, since as you say the
> node is already there?
> 
> Does this affect the primary console device or only secondary ones? Or
> perhaps only secondary ones created using this channels infrastructure? 

It seems to only affect secondary consoles (which includes the channels).

> I'm trying to get a feel for how worried I should be about the potential
> for regressing existing guests with this change.

I believe the ‘type’ frontend key has always been read/only (at least within 
recent memory):

28d386fc        (Ian Jackson    2013-06-25 11:24:22 +0100       3321)    
flexarray_append(ro_front, "type");
ffa165bb        (Ian Campbell   2012-03-01 12:26:14 +0000       3322)    if 
(console->consback == LIBXL__CONSOLE_BACKEND_XENNSOLED)
28d386fc        (Ian Jackson    2013-06-25 11:24:22 +0100       3323)        
flexarray_append(ro_front, "xenconsoled");
5588033d        (Ian Campbell   2010-10-05 17:51:28 +0100       3324)    else
28d386fc        (Ian Jackson    2013-06-25 11:24:22 +0100       3325)        
flexarray_append(ro_front, "ioemu”);

As a sanity check I just fired up a Fedora 20 guest with a single (primary) 
console. In xenstore the key is read-only as expected:

$ sudo xenstore-ls /local/domain/3/console -p
backend = "/local/domain/0/backend/console/3/0"  . . . . . .  (n0,r3)
backend-id = "0" . . . . . . . . . . . . . . . . . . . . . .  (n3,r0)
limit = "1048576"  . . . . . . . . . . . . . . . . . . . . .  (n0,r3)
type = "xenconsoled" . . . . . . . . . . . . . . . . . . . .  (n0,r3)
output = "pty" . . . . . . . . . . . . . . . . . . . . . . .  (n0,r3)
tty = "/dev/pts/1" . . . . . . . . . . . . . . . . . . . . .  (n0,r3)
port = "2" . . . . . . . . . . . . . . . . . . . . . . . . .  (n0,r3)
ring-ref = "1044479" . . . . . . . . . . . . . . . . . . . .  (n0,r3)
vnc-listen = "0.0.0.0" . . . . . . . . . . . . . . . . . . .  (n0,r3)
vnc-port = "5900"  . . . . . . . . . . . . . . . . . . . . .  (n0,r3)

And when I attached ’screen’ to ‘/dev/pts/1’ I found a getty.

I also had a quick look at what pre-libxl xapi does — it seems not to create a 
‘type’ key at all. Primary consoles seem to work ok.

I did a quick rebase and retest of these patches. I’m personally fairly 
confident that nothing bad will happen but I’m a little biased ;-)

Let me know if I should resend. If you want to take a look they’re hosted in 
this branch:

  git pull git://github.com/djs55/xen add-channels10

Cheers,
Dave
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel