Re: [Xen-devel] [PATCH 06/12 v3] xen/arm: vpl011: Add a new vuart node in the xenstore

[Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>]

Hi Wei,

On 11 May 2017 at 16:40, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
> On Wed, May 10, 2017 at 08:05:12PM +0530, Bhupinder Thakur wrote:
>>          libxl__device_console_add(gc, domid, &console, state, &device);
>> @@ -1369,14 +1377,22 @@ static void domcreate_launch_dm(libxl__egc *egc, 
>> libxl__multidev *multidev,
>>      }
>>      case LIBXL_DOMAIN_TYPE_PV:
>>      {
>> -        libxl__device_console console;
>> -        libxl__device device;
>> +        libxl__device_console console, vuart;
>> +        libxl__device device, vuart_device;
>>
>>          for (i = 0; i < d_config->num_vfbs; i++) {
>>              libxl__device_vfb_add(gc, domid, &d_config->vfbs[i]);
>>              libxl__device_vkb_add(gc, domid, &d_config->vkbs[i]);
>>          }
>>
>> +        if (d_config->b_info.vuart)
>> +        {
>> +            init_console_info(gc, &vuart, 0);
>> +            vuart.backend_domid = state->console_domid;
>> +            libxl__device_vuart_add(gc, domid, &vuart, state, 
>> &vuart_device);
>> +            libxl__device_console_dispose(&vuart);
>> +        }
>> +
>
> This is strange. You have code snippet for both PV and HVM. Why?
Yes I believe I should have added only for PV guest but to keep the
code consistent for both type of guests, I added for both.
I will remove it for HVM.

>
>>          init_console_info(gc, &console, 0);
>>          console.backend_domid = state->console_domid;
>>          libxl__device_console_add(gc, domid, &console, state, &device);
>> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
>> index 5e96676..bb25672 100644
>> --- a/tools/libxl/libxl_device.c
>> +++ b/tools/libxl/libxl_device.c
>> @@ -26,6 +26,9 @@ static char *libxl__device_frontend_path(libxl__gc *gc, 
>> libxl__device *device)
>>      if (device->kind == LIBXL__DEVICE_KIND_CONSOLE && device->devid == 0)
>>          return GCSPRINTF("%s/console", dom_path);
>>
>> +    if (device->kind == LIBXL__DEVICE_KIND_VUART)
>> +        return GCSPRINTF("%s/vuart/%d", dom_path, device->devid);
>> +
>>      return GCSPRINTF("%s/device/%s/%d", dom_path,
>>                       libxl__device_kind_to_string(device->kind),
>>                       device->devid);
>> @@ -151,13 +154,19 @@ retry_transaction:
>>      if (rc) goto out;
>>
>>      if (!libxl_only) {
>> -        rc = libxl__xs_write_checked(gc, t, 
>> GCSPRINTF("%s/frontend",libxl_path),
>> -                                     frontend_path);
>> -        if (rc) goto out;
>> +        if (fents || ro_fents)
>> +        {
>> +            rc = libxl__xs_write_checked(gc, t, 
>> GCSPRINTF("%s/frontend",libxl_path),
>> +                                         frontend_path);
>> +            if (rc) goto out;
>> +        }
>>
>> -        rc = libxl__xs_write_checked(gc, t, 
>> GCSPRINTF("%s/backend",libxl_path),
>> -                                     backend_path);
>> -        if (rc) goto out;
>> +        if (bents)
>> +        {
>> +            rc = libxl__xs_write_checked(gc, t, 
>> GCSPRINTF("%s/backend",libxl_path),
>> +                                         backend_path);
>> +            if (rc) goto out;
>> +        }
>
> What is this for?
>
> If there is no fe or be entries you skip the path creation altogether.
> But why? This doesn't seem to be related to your patch.
For vuart, I am adding only a front end node but the
libxl__device_generic_add() creates the backend path also,even though
there is no backend node. To remove that hanging be path, I added this
check.
>
> At least explain this a bit in the commit message?
I will add more details in the commit message.

Regards,
Bhupinder

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel