| Message ID | 1493395284-18430-7-git-send-email-bhupinder.thakur@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 28 Apr 2017, Bhupinder Thakur wrote: > 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(+) > > diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c > index 446e766..ef3bd44 100644 > --- a/tools/libxl/libxl_console.c > +++ b/tools/libxl/libxl_console.c > @@ -67,6 +67,9 @@ int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, > case LIBXL_CONSOLE_TYPE_SERIAL: > cons_type_s = "serial"; > break; > + case LIBXL_CONSOLE_TYPE_VUART: > + cons_type_s = "vuart"; > + break; > default: > goto out; > } > @@ -326,6 +329,13 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid, > flexarray_append(ro_front, GCSPRINTF("%"PRIu32, state->console_port)); > flexarray_append(ro_front, "ring-ref"); > flexarray_append(ro_front, GCSPRINTF("%lu", state->console_mfn)); > + if (state->vuart_enabled) > + { > + flexarray_append(ro_front, "vuart/0/port"); > + flexarray_append(ro_front, GCSPRINTF("%"PRIu32, state->vuart_port)); > + flexarray_append(ro_front, "vuart/0/ring-ref"); > + flexarray_append(ro_front, GCSPRINTF("%lu", state->vuart_mfn)); > + } 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. > } else { > flexarray_append(front, "state"); > flexarray_append(front, GCSPRINTF("%d", XenbusStateInitialising)); > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index 5d914a5..06ff3b7 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -434,6 +434,9 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, > state->store_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->store_domid); > state->console_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->console_domid); > > + if (state->vuart_enabled) > + xc_domain_vuart_get_evtchn(ctx->xch, domid, &state->vuart_port); > + > if (info->type == LIBXL_DOMAIN_TYPE_HVM) { > hvm_set_conf_params(ctx->xch, domid, info); > #if defined(__i386__) || defined(__x86_64__) > @@ -788,6 +791,7 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid, > if (xc_dom_translated(dom)) { > state->console_mfn = dom->console_pfn; > state->store_mfn = dom->xenstore_pfn; > + state->vuart_mfn = dom->vuart_pfn; > } else { > state->console_mfn = xc_dom_p2m(dom, dom->console_pfn); > state->store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn); These two changes to libxl_dom.c probably belong to patch #4
Hi Stefano, >> diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c >> index 446e766..ef3bd44 100644 >> --- a/tools/libxl/libxl_console.c >> +++ b/tools/libxl/libxl_console.c >> @@ -67,6 +67,9 @@ int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, >> case LIBXL_CONSOLE_TYPE_SERIAL: >> cons_type_s = "serial"; >> break; >> + case LIBXL_CONSOLE_TYPE_VUART: >> + cons_type_s = "vuart"; >> + break; >> default: >> goto out; >> } >> @@ -326,6 +329,13 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid, >> flexarray_append(ro_front, GCSPRINTF("%"PRIu32, state->console_port)); >> flexarray_append(ro_front, "ring-ref"); >> flexarray_append(ro_front, GCSPRINTF("%lu", state->console_mfn)); >> + if (state->vuart_enabled) >> + { >> + flexarray_append(ro_front, "vuart/0/port"); >> + flexarray_append(ro_front, GCSPRINTF("%"PRIu32, state->vuart_port)); >> + flexarray_append(ro_front, "vuart/0/ring-ref"); >> + flexarray_append(ro_front, GCSPRINTF("%lu", state->vuart_mfn)); >> + } > > 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. > >> } else { >> flexarray_append(front, "state"); >> flexarray_append(front, GCSPRINTF("%d", XenbusStateInitialising)); >> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c >> index 5d914a5..06ff3b7 100644 >> --- a/tools/libxl/libxl_dom.c >> +++ b/tools/libxl/libxl_dom.c >> @@ -434,6 +434,9 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, >> state->store_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->store_domid); >> state->console_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->console_domid); >> >> + if (state->vuart_enabled) >> + xc_domain_vuart_get_evtchn(ctx->xch, domid, &state->vuart_port); >> + >> if (info->type == LIBXL_DOMAIN_TYPE_HVM) { >> hvm_set_conf_params(ctx->xch, domid, info); >> #if defined(__i386__) || defined(__x86_64__) >> @@ -788,6 +791,7 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid, >> if (xc_dom_translated(dom)) { >> state->console_mfn = dom->console_pfn; >> state->store_mfn = dom->xenstore_pfn; >> + state->vuart_mfn = dom->vuart_pfn; >> } else { >> state->console_mfn = xc_dom_p2m(dom, dom->console_pfn); >> state->store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn); > > These two changes to libxl_dom.c probably belong to patch #4 Ok. I will move these changes to the patch#4. Regards, Bhupinder
On Mon, 1 May 2017, Bhupinder Thakur wrote: > Hi Stefano, > > >> diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c > >> index 446e766..ef3bd44 100644 > >> --- a/tools/libxl/libxl_console.c > >> +++ b/tools/libxl/libxl_console.c > >> @@ -67,6 +67,9 @@ int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, > >> case LIBXL_CONSOLE_TYPE_SERIAL: > >> cons_type_s = "serial"; > >> break; > >> + case LIBXL_CONSOLE_TYPE_VUART: > >> + cons_type_s = "vuart"; > >> + break; > >> default: > >> goto out; > >> } > >> @@ -326,6 +329,13 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid, > >> flexarray_append(ro_front, GCSPRINTF("%"PRIu32, state->console_port)); > >> flexarray_append(ro_front, "ring-ref"); > >> flexarray_append(ro_front, GCSPRINTF("%lu", state->console_mfn)); > >> + if (state->vuart_enabled) > >> + { > >> + flexarray_append(ro_front, "vuart/0/port"); > >> + flexarray_append(ro_front, GCSPRINTF("%"PRIu32, state->vuart_port)); > >> + flexarray_append(ro_front, "vuart/0/ring-ref"); > >> + flexarray_append(ro_front, GCSPRINTF("%lu", state->vuart_mfn)); > >> + } > > > > 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. > >> } else { > >> flexarray_append(front, "state"); > >> flexarray_append(front, GCSPRINTF("%d", XenbusStateInitialising)); > >> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > >> index 5d914a5..06ff3b7 100644 > >> --- a/tools/libxl/libxl_dom.c > >> +++ b/tools/libxl/libxl_dom.c > >> @@ -434,6 +434,9 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, > >> state->store_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->store_domid); > >> state->console_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->console_domid); > >> > >> + if (state->vuart_enabled) > >> + xc_domain_vuart_get_evtchn(ctx->xch, domid, &state->vuart_port); > >> + > >> if (info->type == LIBXL_DOMAIN_TYPE_HVM) { > >> hvm_set_conf_params(ctx->xch, domid, info); > >> #if defined(__i386__) || defined(__x86_64__) > >> @@ -788,6 +791,7 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid, > >> if (xc_dom_translated(dom)) { > >> state->console_mfn = dom->console_pfn; > >> state->store_mfn = dom->xenstore_pfn; > >> + state->vuart_mfn = dom->vuart_pfn; > >> } else { > >> state->console_mfn = xc_dom_p2m(dom, dom->console_pfn); > >> state->store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn); > > > > These two changes to libxl_dom.c probably belong to patch #4 > Ok. I will move these changes to the patch#4. > > Regards, > Bhupinder >
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
On Fri, 5 May 2017, Bhupinder Thakur wrote: > 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? > > > > > >> 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)
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
diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c index 446e766..ef3bd44 100644 --- a/tools/libxl/libxl_console.c +++ b/tools/libxl/libxl_console.c @@ -67,6 +67,9 @@ int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, case LIBXL_CONSOLE_TYPE_SERIAL: cons_type_s = "serial"; break; + case LIBXL_CONSOLE_TYPE_VUART: + cons_type_s = "vuart"; + break; default: goto out; } @@ -326,6 +329,13 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid, flexarray_append(ro_front, GCSPRINTF("%"PRIu32, state->console_port)); flexarray_append(ro_front, "ring-ref"); flexarray_append(ro_front, GCSPRINTF("%lu", state->console_mfn)); + if (state->vuart_enabled) + { + flexarray_append(ro_front, "vuart/0/port"); + flexarray_append(ro_front, GCSPRINTF("%"PRIu32, state->vuart_port)); + flexarray_append(ro_front, "vuart/0/ring-ref"); + flexarray_append(ro_front, GCSPRINTF("%lu", state->vuart_mfn)); + } } else { flexarray_append(front, "state"); flexarray_append(front, GCSPRINTF("%d", XenbusStateInitialising)); diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index 5d914a5..06ff3b7 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -434,6 +434,9 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, state->store_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->store_domid); state->console_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->console_domid); + if (state->vuart_enabled) + xc_domain_vuart_get_evtchn(ctx->xch, domid, &state->vuart_port); + if (info->type == LIBXL_DOMAIN_TYPE_HVM) { hvm_set_conf_params(ctx->xch, domid, info); #if defined(__i386__) || defined(__x86_64__) @@ -788,6 +791,7 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid, if (xc_dom_translated(dom)) { state->console_mfn = dom->console_pfn; state->store_mfn = dom->xenstore_pfn; + state->vuart_mfn = dom->vuart_pfn; } else { state->console_mfn = xc_dom_p2m(dom, dom->console_pfn); state->store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn);
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(+)