Re: [Xen-devel] [PATCH v3 04/11] libxl: add generic function to add device

[Oleksandr Grytsov <al1img@xxxxxxxxx>]

On Fri, Jul 7, 2017 at 1:56 PM, Oleksandr Grytsov <al1img@xxxxxxxxx> wrote:
> On Fri, Jul 7, 2017 at 1:32 PM, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
>> On Fri, Jul 07, 2017 at 01:29:39PM +0300, Oleksandr Grytsov wrote:
>>  > Actually my the first patch probably was done on the old codebase
>>> > which doesn't have locking in add function. So new approach is
>>> > definitely wrong and I will use former one.
>>>
>>> Please ignore my above comment. Actually it looks like my new approach
>>> changes former behavior. I will rework this function to match former one.
>>>
>>> Actually new approach
>>
>> Hit "Send" too soon?
>
> Just forgot to remove this line. So, I will rework this part.
>

Few questions about former implementation (I address vtpm as reference
but questions are related to all devices):

1. Using of libxl_device_vtpm vtpm_saved variable. It is unclear why
we need this additional variable.
There is no any rollback or cancellation with this variable.
It is used to be added to the domain config but vtpm from input
parameter can be used for this reason as well.

2. Why libxl__update_config_vtpm(gc, &vtpm_saved, vtpm); is called if
just before we copied
vtpm_saved from vtpm?

    libxl_device_vtpm_init(&vtpm_saved);
    libxl_device_vtpm_copy(CTX, &vtpm_saved, vtpm);

I see that dev id is updated but it could be done before copy operation.

3. What is reason to call libxl__set_domain_configuration(gc, domid,
&d_config); in each
xen store transaction attempt?

Thanks.

> --
> Best Regards,
> Oleksandr Grytsov.



-- 
Best Regards,
Oleksandr Grytsov.

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