Commit Message

Add two new parameters to the xen store to be used by xenconsoled:
- newly allocated PFN to be used as IN/OUT ring buffer
- get a new event channel allocated by Xen using a domctl call
These paramters are added to xenstore only if vuart console is enabled
by the user.
Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
---
Changes since v1:
- Modified the xenstore key names to /vuart/0/ring-ref and
/vuart/0/port.
- Replaced the hvm call with domctl call to get the event channel.
tools/libxl/libxl_console.c | 10 ++++++++++
tools/libxl/libxl_dom.c | 4 ++++
2 files changed, 14 insertions(+)

Hi,
>> > It looks like you are reusing the libxl__device_console_add call for the>> > main PV console for the domain, to also add the vuart nodes to xenstore.>> >>> > I don't think it is a good idea to mix the two. I suggest to introduce a>> > new libxl__device call to introduce the vuart nodes to xenstore, given>> > that they have no relantionship with the principal PV console of the>> > domain.>> >>> I have a doubt here. Do I have to create a new console device>> (libxl__device) to register the vuart console or can I use the>> existing console device (which is used for registering the primary>> console) to register the vuart nodes to xenstore?>>>> I suspect that if I try to register with the same console device then>> libxl__device_generic_add() may fail as it is already added.>> The vuart we are introducing with this patch series is a new and> different console from the existing PV console. So yes, I think we need> to create a new device for it.>> We either need to introduce a brand new function to add the vuart to> xenstore, something like libxl__device_vuart_add, or, if we are going> to reuse libxl__device_console_add, then we we need to create both a new> console device (libxl__device_console), and a new libxl__device for it.> For clarity, libxl__device_console is the input parameter of> libxl__device_console_add, while libxl__device is the output parameter.>
I am trying to add a new vuart device by defining a new device type
LIBXL__DEVICE_KIND_VUART and a new function
libxl__device_vuart_add().
This function basically adds the vuart ring-ref ("0/ring-ref") and
port ("0/port") to ro_front array and then it registers the device
using the libxl__device_generic_add(). I pass NULL to "back" and
"front" array arguments to libxl__device_generic_add() as I am adding
parameters only to "ro_front" array.
I am adding the device at /local/domain/<domid>/vuart.
However, xenconsoled fails to read the vuart ring-ref and port saying
"no such directory or file".I verified that
libxl__device_generic_add() is returning successfully.
In xenconsoled, I verified that I am reading
/local/domain/<domid>/vuart/0/ring-ref and
/local/domain/<domid>/vuart/0/port.
It is not clear why xenconsoled fails to read the values.
Regards,
Bhupinder

On Wed, 3 May 2017, Bhupinder Thakur wrote:
> Hi,> > >> > It looks like you are reusing the libxl__device_console_add call for the> >> > main PV console for the domain, to also add the vuart nodes to xenstore.> >> >> >> > I don't think it is a good idea to mix the two. I suggest to introduce a> >> > new libxl__device call to introduce the vuart nodes to xenstore, given> >> > that they have no relantionship with the principal PV console of the> >> > domain.> >> >> >> I have a doubt here. Do I have to create a new console device> >> (libxl__device) to register the vuart console or can I use the> >> existing console device (which is used for registering the primary> >> console) to register the vuart nodes to xenstore?> >>> >> I suspect that if I try to register with the same console device then> >> libxl__device_generic_add() may fail as it is already added.> >> > The vuart we are introducing with this patch series is a new and> > different console from the existing PV console. So yes, I think we need> > to create a new device for it.> >> > We either need to introduce a brand new function to add the vuart to> > xenstore, something like libxl__device_vuart_add, or, if we are going> > to reuse libxl__device_console_add, then we we need to create both a new> > console device (libxl__device_console), and a new libxl__device for it.> > For clarity, libxl__device_console is the input parameter of> > libxl__device_console_add, while libxl__device is the output parameter.> >> I am trying to add a new vuart device by defining a new device type> LIBXL__DEVICE_KIND_VUART and a new function> libxl__device_vuart_add().
I think that is the right way to do it.
> This function basically adds the vuart ring-ref ("0/ring-ref") and> port ("0/port") to ro_front array and then it registers the device> using the libxl__device_generic_add(). I pass NULL to "back" and> "front" array arguments to libxl__device_generic_add() as I am adding> parameters only to "ro_front" array.> > I am adding the device at /local/domain/<domid>/vuart.> > However, xenconsoled fails to read the vuart ring-ref and port saying> "no such directory or file".I verified that> libxl__device_generic_add() is returning successfully.> > In xenconsoled, I verified that I am reading> /local/domain/<domid>/vuart/0/ring-ref and> /local/domain/<domid>/vuart/0/port.> > It is not clear why xenconsoled fails to read the values.
Double check the permissions of those nodes with xenstore-ls -p. Make
sure there are no races between libxl writing the nodes and xenconsoled
reading them.

Hi,
>> >> > It looks like you are reusing the libxl__device_console_add call for the>> >> > main PV console for the domain, to also add the vuart nodes to xenstore.>> >> >>> >> > I don't think it is a good idea to mix the two. I suggest to introduce a>> >> > new libxl__device call to introduce the vuart nodes to xenstore, given>> >> > that they have no relantionship with the principal PV console of the>> >> > domain.>> >> >>> >> I have a doubt here. Do I have to create a new console device>> >> (libxl__device) to register the vuart console or can I use the>> >> existing console device (which is used for registering the primary>> >> console) to register the vuart nodes to xenstore?>> >>>> >> I suspect that if I try to register with the same console device then>> >> libxl__device_generic_add() may fail as it is already added.>> >>> > The vuart we are introducing with this patch series is a new and>> > different console from the existing PV console. So yes, I think we need>> > to create a new device for it.>> >>> > We either need to introduce a brand new function to add the vuart to>> > xenstore, something like libxl__device_vuart_add, or, if we are going>> > to reuse libxl__device_console_add, then we we need to create both a new>> > console device (libxl__device_console), and a new libxl__device for it.>> > For clarity, libxl__device_console is the input parameter of>> > libxl__device_console_add, while libxl__device is the output parameter.>> >>> I am trying to add a new vuart device by defining a new device type>> LIBXL__DEVICE_KIND_VUART and a new function>> libxl__device_vuart_add().>> I think that is the right way to do it.>>>> This function basically adds the vuart ring-ref ("0/ring-ref") and>> port ("0/port") to ro_front array and then it registers the device>> using the libxl__device_generic_add(). I pass NULL to "back" and>> "front" array arguments to libxl__device_generic_add() as I am adding>> parameters only to "ro_front" array.>>>> I am adding the device at /local/domain/<domid>/vuart.>>>> However, xenconsoled fails to read the vuart ring-ref and port saying>> "no such directory or file".I verified that>> libxl__device_generic_add() is returning successfully.>>>> In xenconsoled, I verified that I am reading>> /local/domain/<domid>/vuart/0/ring-ref and>> /local/domain/<domid>/vuart/0/port.>>>> It is not clear why xenconsoled fails to read the values.>> Double check the permissions of those nodes with xenstore-ls -p. Make> sure there are no races between libxl writing the nodes and xenconsoled> reading them.
xenstore-ls -p" shows that the vuart node is created successfully. I
have pasted below the relevant part of the command output:
vuart = "" (n0,r1)
backend = "/local/domain/0/backend/vuart/1/0" (n0,r1)
0 = "" (n0,r1)
port = "1" (n0,r1)
ring-ref = "233475" (n0,r1)
limit = "1048576" (n0,r1)
type = "xenconsoled" (n0,r1)
console = "" (n0,r1)
backend = "/local/domain/0/backend/console/1/0" (n0,r1)
backend-id = "0" (n1,r0)
limit = "1048576" (n0,r1)
type = "xenconsoled" (n0,r1)
output = "pty" (n0,r1)
tty = "/dev/pts/0" (n0,r1)
port = "3" (n0,r1)
ring-ref = "233472" (n0,r1)
The vuart node exists at /local/domain/1/vuart. I am creating the
vuart node just before the pv console node to make sure that vuart
node becomes visible before the pv console node is created. The values
are populated properly in the vuart node. Still when xenconsoled tries
to read /loca/domain/1/vuart/0/ring-ref or
/loca/domain/1/vuart/0/port, the read fails.
PV console node reads happen properly even though both nodes exist at
the same level and their access permissions are also same.
Regards,
Bhupinder

On Fri, 5 May 2017, Bhupinder Thakur wrote:
> Hi,> > > >> >> > It looks like you are reusing the libxl__device_console_add call for the> >> >> > main PV console for the domain, to also add the vuart nodes to xenstore.> >> >> >> >> >> > I don't think it is a good idea to mix the two. I suggest to introduce a> >> >> > new libxl__device call to introduce the vuart nodes to xenstore, given> >> >> > that they have no relantionship with the principal PV console of the> >> >> > domain.> >> >> >> >> >> I have a doubt here. Do I have to create a new console device> >> >> (libxl__device) to register the vuart console or can I use the> >> >> existing console device (which is used for registering the primary> >> >> console) to register the vuart nodes to xenstore?> >> >>> >> >> I suspect that if I try to register with the same console device then> >> >> libxl__device_generic_add() may fail as it is already added.> >> >> >> > The vuart we are introducing with this patch series is a new and> >> > different console from the existing PV console. So yes, I think we need> >> > to create a new device for it.> >> >> >> > We either need to introduce a brand new function to add the vuart to> >> > xenstore, something like libxl__device_vuart_add, or, if we are going> >> > to reuse libxl__device_console_add, then we we need to create both a new> >> > console device (libxl__device_console), and a new libxl__device for it.> >> > For clarity, libxl__device_console is the input parameter of> >> > libxl__device_console_add, while libxl__device is the output parameter.> >> >> >> I am trying to add a new vuart device by defining a new device type> >> LIBXL__DEVICE_KIND_VUART and a new function> >> libxl__device_vuart_add().> >> > I think that is the right way to do it.> >> >> >> This function basically adds the vuart ring-ref ("0/ring-ref") and> >> port ("0/port") to ro_front array and then it registers the device> >> using the libxl__device_generic_add(). I pass NULL to "back" and> >> "front" array arguments to libxl__device_generic_add() as I am adding> >> parameters only to "ro_front" array.> >>> >> I am adding the device at /local/domain/<domid>/vuart.> >>> >> However, xenconsoled fails to read the vuart ring-ref and port saying> >> "no such directory or file".I verified that> >> libxl__device_generic_add() is returning successfully.> >>> >> In xenconsoled, I verified that I am reading> >> /local/domain/<domid>/vuart/0/ring-ref and> >> /local/domain/<domid>/vuart/0/port.> >>> >> It is not clear why xenconsoled fails to read the values.> >> > Double check the permissions of those nodes with xenstore-ls -p. Make> > sure there are no races between libxl writing the nodes and xenconsoled> > reading them.> > xenstore-ls -p" shows that the vuart node is created successfully. I> have pasted below the relevant part of the command output:> > vuart = "" (n0,r1)> backend = "/local/domain/0/backend/vuart/1/0" (n0,r1)
Why is this here instead of under vuart/0?
Also, if these are the frontend nodes, where are the backend vuart nodes?
> 0 = "" (n0,r1)> port = "1" (n0,r1)> ring-ref = "233475" (n0,r1)> limit = "1048576" (n0,r1)> type = "xenconsoled" (n0,r1)> console = "" (n0,r1)> backend = "/local/domain/0/backend/console/1/0" (n0,r1)> backend-id = "0" (n1,r0)> limit = "1048576" (n0,r1)> type = "xenconsoled" (n0,r1)> output = "pty" (n0,r1)> tty = "/dev/pts/0" (n0,r1)> port = "3" (n0,r1)> ring-ref = "233472" (n0,r1)> > The vuart node exists at /local/domain/1/vuart. I am creating the> vuart node just before the pv console node to make sure that vuart> node becomes visible before the pv console node is created. The values> are populated properly in the vuart node. Still when xenconsoled tries> to read /loca/domain/1/vuart/0/ring-ref or> /loca/domain/1/vuart/0/port, the read fails.> > PV console node reads happen properly even though both nodes exist at> the same level and their access permissions are also same.
I don't know. I would add more debug printfs to the code until I
understood what was going on.

Hi Stefano,
>> >> >> > It looks like you are reusing the libxl__device_console_add call for the>> >> >> > main PV console for the domain, to also add the vuart nodes to xenstore.>> >> >> >>> >> >> > I don't think it is a good idea to mix the two. I suggest to introduce a>> >> >> > new libxl__device call to introduce the vuart nodes to xenstore, given>> >> >> > that they have no relantionship with the principal PV console of the>> >> >> > domain.>> >> >> >>> >> >> I have a doubt here. Do I have to create a new console device>> >> >> (libxl__device) to register the vuart console or can I use the>> >> >> existing console device (which is used for registering the primary>> >> >> console) to register the vuart nodes to xenstore?>> >> >>>> >> >> I suspect that if I try to register with the same console device then>> >> >> libxl__device_generic_add() may fail as it is already added.>> >> >>> >> > The vuart we are introducing with this patch series is a new and>> >> > different console from the existing PV console. So yes, I think we need>> >> > to create a new device for it.>> >> >>> >> > We either need to introduce a brand new function to add the vuart to>> >> > xenstore, something like libxl__device_vuart_add, or, if we are going>> >> > to reuse libxl__device_console_add, then we we need to create both a new>> >> > console device (libxl__device_console), and a new libxl__device for it.>> >> > For clarity, libxl__device_console is the input parameter of>> >> > libxl__device_console_add, while libxl__device is the output parameter.>> >> >>> >> I am trying to add a new vuart device by defining a new device type>> >> LIBXL__DEVICE_KIND_VUART and a new function>> >> libxl__device_vuart_add().>> >>> > I think that is the right way to do it.>> >>> >>> >> This function basically adds the vuart ring-ref ("0/ring-ref") and>> >> port ("0/port") to ro_front array and then it registers the device>> >> using the libxl__device_generic_add(). I pass NULL to "back" and>> >> "front" array arguments to libxl__device_generic_add() as I am adding>> >> parameters only to "ro_front" array.>> >>>> >> I am adding the device at /local/domain/<domid>/vuart.>> >>>> >> However, xenconsoled fails to read the vuart ring-ref and port saying>> >> "no such directory or file".I verified that>> >> libxl__device_generic_add() is returning successfully.>> >>>> >> In xenconsoled, I verified that I am reading>> >> /local/domain/<domid>/vuart/0/ring-ref and>> >> /local/domain/<domid>/vuart/0/port.>> >>>> >> It is not clear why xenconsoled fails to read the values.>> >>> > Double check the permissions of those nodes with xenstore-ls -p. Make>> > sure there are no races between libxl writing the nodes and xenconsoled>> > reading them.>>>> xenstore-ls -p" shows that the vuart node is created successfully. I>> have pasted below the relevant part of the command output:>>>> vuart = "" (n0,r1)>> backend = "/local/domain/0/backend/vuart/1/0" (n0,r1)>> Why is this here instead of under vuart/0?> Also, if these are the frontend nodes, where are the backend vuart nodes?
I created only frontend nodes so that xenconsoled could access
ring-ref/port. I understand that backend nodes are required for guest
domains to access some backend information. Since in this case guest
domain does not need such information I did not create the backend
node. Kindly let me know if my understanding is correct.
>>>> 0 = "" (n0,r1)>> port = "1" (n0,r1)>> ring-ref = "233475" (n0,r1)>> limit = "1048576" (n0,r1)>> type = "xenconsoled" (n0,r1)>> console = "" (n0,r1)>> backend = "/local/domain/0/backend/console/1/0" (n0,r1)>> backend-id = "0" (n1,r0)>> limit = "1048576" (n0,r1)>> type = "xenconsoled" (n0,r1)>> output = "pty" (n0,r1)>> tty = "/dev/pts/0" (n0,r1)>> port = "3" (n0,r1)>> ring-ref = "233472" (n0,r1)>>>> The vuart node exists at /local/domain/1/vuart. I am creating the>> vuart node just before the pv console node to make sure that vuart>> node becomes visible before the pv console node is created. The values>> are populated properly in the vuart node. Still when xenconsoled tries>> to read /loca/domain/1/vuart/0/ring-ref or>> /loca/domain/1/vuart/0/port, the read fails.>>>> PV console node reads happen properly even though both nodes exist at>> the same level and their access permissions are also same.>> I don't know. I would add more debug printfs to the code until I> understood what was going on.
There was one issue in domain_create_ring() where if the vuart node
was populated later then it was not able to be open the tty session
since the check was based on whether pv tty was initialized (which was
initialized earlier).
Earlier, this was not the case since vuart and pv xenstore parameters
were always available together.
I have defined a new function console_create_ring() which is called
for both consoles. This new function removes lot of duplicate code for
pv and vuart consoles in domain_create_ring(). With this change, both
the consoles are working fine now.
Regards,
Bhupinder

Hi Stefano,
>> >> >> >> > It looks like you are reusing the libxl__device_console_add call for the>> >> >> >> > main PV console for the domain, to also add the vuart nodes to xenstore.>> >> >> >> >>> >> >> >> > I don't think it is a good idea to mix the two. I suggest to introduce a>> >> >> >> > new libxl__device call to introduce the vuart nodes to xenstore, given>> >> >> >> > that they have no relantionship with the principal PV console of the>> >> >> >> > domain.>> >> >> >> >>> >> >> >> I have a doubt here. Do I have to create a new console device>> >> >> >> (libxl__device) to register the vuart console or can I use the>> >> >> >> existing console device (which is used for registering the primary>> >> >> >> console) to register the vuart nodes to xenstore?>> >> >> >>>> >> >> >> I suspect that if I try to register with the same console device then>> >> >> >> libxl__device_generic_add() may fail as it is already added.>> >> >> >>> >> >> > The vuart we are introducing with this patch series is a new and>> >> >> > different console from the existing PV console. So yes, I think we need>> >> >> > to create a new device for it.>> >> >> >>> >> >> > We either need to introduce a brand new function to add the vuart to>> >> >> > xenstore, something like libxl__device_vuart_add, or, if we are going>> >> >> > to reuse libxl__device_console_add, then we we need to create both a new>> >> >> > console device (libxl__device_console), and a new libxl__device for it.>> >> >> > For clarity, libxl__device_console is the input parameter of>> >> >> > libxl__device_console_add, while libxl__device is the output parameter.>> >> >> >>> >> >> I am trying to add a new vuart device by defining a new device type>> >> >> LIBXL__DEVICE_KIND_VUART and a new function>> >> >> libxl__device_vuart_add().>> >> >>> >> > I think that is the right way to do it.>> >> >>> >> >>> >> >> This function basically adds the vuart ring-ref ("0/ring-ref") and>> >> >> port ("0/port") to ro_front array and then it registers the device>> >> >> using the libxl__device_generic_add(). I pass NULL to "back" and>> >> >> "front" array arguments to libxl__device_generic_add() as I am adding>> >> >> parameters only to "ro_front" array.>> >> >>>> >> >> I am adding the device at /local/domain/<domid>/vuart.>> >> >>>> >> >> However, xenconsoled fails to read the vuart ring-ref and port saying>> >> >> "no such directory or file".I verified that>> >> >> libxl__device_generic_add() is returning successfully.>> >> >>>> >> >> In xenconsoled, I verified that I am reading>> >> >> /local/domain/<domid>/vuart/0/ring-ref and>> >> >> /local/domain/<domid>/vuart/0/port.>> >> >>>> >> >> It is not clear why xenconsoled fails to read the values.>> >> >>> >> > Double check the permissions of those nodes with xenstore-ls -p. Make>> >> > sure there are no races between libxl writing the nodes and xenconsoled>> >> > reading them.>> >>>> >> xenstore-ls -p" shows that the vuart node is created successfully. I>> >> have pasted below the relevant part of the command output:>> >>>> >> vuart = "" (n0,r1)>> >> backend = "/local/domain/0/backend/vuart/1/0" (n0,r1)>> >>> > Why is this here instead of under vuart/0?>> > Also, if these are the frontend nodes, where are the backend vuart nodes?>>>> I created only frontend nodes so that xenconsoled could access>> ring-ref/port. I understand that backend nodes are required for guest>> domains to access some backend information. Since in this case guest>> domain does not need such information I did not create the backend>> node. Kindly let me know if my understanding is correct.>> If there are no backend xenstore nodes, why do we have:>> backend = "/local/domain/0/backend/vuart/1/0">> on Xenstore?>
libxl__device_generic_add() creates the references to backend and
frontend even if those nodes are not created (i.e. the node
information passed to the function are null). I will add a check in
libxl__device_generic_add() to add these references only if the
corresponding nodes have been created in xenstore.
Regards,
Bhupinder