Re: [Xen-devel] [PATCH 15/15] xen: Rename xen_be_frontend_changed

[Paolo Bonzini]

> So we are better leave it as is. Maybe we need to rename some functions in
> that file.
>  __iirc__ adding xen_frontend.c is one of Stefano's comments in previous v6
>  or v7..

I agree; it's quite possible that code can be shared between backend and
frontend (renaming functions as needed), and it's good to have a generic
xen frontend mechanism instead of something specific to TPM.

Paolo

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

Re: [Xen-devel] [PATCH 15/15] xen: Rename xen_be_frontend_changed

[Xuquan (Quan Xu)]

On October 13, 2016 2:09 PM, Emil Condrea  wrote:
>As you suggested, I've dropped the all patches for xen_frontend.
>
>Emil
>
>On Wed, Oct 12, 2016 at 2:00 PM, Paolo Bonzini  wrote:
>>
>>
>> On 09/10/2016 21:50, Emil Condrea wrote:
>>> On Tue, Oct 4, 2016 at 11:06 AM, Paolo Bonzini 
>wrote:


 On 04/10/2016 08:43, Emil Condrea wrote:
> xen_be_frontend_changed -> xen_fe_frontend_changed

 This is not correct.  The front-end is implemented in the guest
 domain, while the back-end is implemented in the dom0 or stubdom.

>>>


Hi Paolo, 

Yes, the front-end is almost implemented in the guest 
This case (as mentioned in 00/15) is very particular. The frontend is actually 
implemented in QEMU, and the guest still run native driver tpm_tis.ko..


>>> You are right, thanks for the feedback! I will drop this patch
>>> together with the hunk from 04/15 patch which moves this function to
>>> xen_frontend.c
>>
>> Actually all of your new xen_frontend.c seems to be reading frontend
>> information from XenStore, which is typically something that the
>> backend does.  So I suggest dropping the patch altogether.
>>

So we are better leave it as is. Maybe we need to rename some functions in that 
file.
 __iirc__ adding xen_frontend.c is one of Stefano's comments in previous v6 or 
v7.. 

Quan

>> Thanks,
>>
>> Paolo
>>
 This function processes *in the backed* a notification that the
 frontend state changed, hence the name should be
>xen_be_frontend_changed.

 Paolo

> Signed-off-by: Emil Condrea 
> ---
>  hw/xen/xen_backend.c  | 2 +-
>  hw/xen/xen_frontend.c | 4 ++--
>  include/hw/xen/xen_frontend.h | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c index
> 30d3aaa..b79e83e 100644
> --- a/hw/xen/xen_backend.c
> +++ b/hw/xen/xen_backend.c
> @@ -213,7 +213,7 @@ static int xen_be_try_setup(struct XenDevice
>*xendev)
>  xen_be_set_state(xendev, XenbusStateInitialising);
>
>  xen_be_backend_changed(xendev, NULL);
> -xen_be_frontend_changed(xendev, NULL);
> +xen_fe_frontend_changed(xendev, NULL);
>  return 0;
>  }
>
> diff --git a/hw/xen/xen_frontend.c b/hw/xen/xen_frontend.c index
> 1407f5f..761688b 100644
> --- a/hw/xen/xen_frontend.c
> +++ b/hw/xen/xen_frontend.c
> @@ -39,7 +39,7 @@ int xenstore_read_fe_uint64(struct XenDevice
>*xendev, const char *node,
>  return xenstore_read_uint64(xendev->fe, node, uval);  }
>
> -void xen_be_frontend_changed(struct XenDevice *xendev, const char
> *node)
> +void xen_fe_frontend_changed(struct XenDevice *xendev, const char
> +*node)
>  {
>  int fe_state;
>
> @@ -85,6 +85,6 @@ void xenstore_update_fe(char *watch, struct
>XenDevice *xendev)
>  }
>  node = watch + len + 1;
>
> -xen_be_frontend_changed(xendev, node);
> +xen_fe_frontend_changed(xendev, node);
>  xen_be_check_state(xendev);
>  }
> diff --git a/include/hw/xen/xen_frontend.h
> b/include/hw/xen/xen_frontend.h index bb0bc23..2a5f03f 100644
> --- a/include/hw/xen/xen_frontend.h
> +++ b/include/hw/xen/xen_frontend.h
> @@ -9,6 +9,6 @@ int xenstore_read_fe_uint64(struct XenDevice *xendev,
>const char *node,
>
>uint64_t
> *uval);  void xenstore_update_fe(char *watch, struct XenDevice
> *xendev);
>
> -void xen_be_frontend_changed(struct XenDevice *xendev, const char
> *node);
> +void xen_fe_frontend_changed(struct XenDevice *xendev, const char
> +*node);
>
>  #endif /* QEMU_HW_XEN_FRONTEND_H */
>
>>>
>>>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 15/15] xen: Rename xen_be_frontend_changed

[Emil Condrea]

As you suggested, I've dropped the all patches for xen_frontend.

Emil

On Wed, Oct 12, 2016 at 2:00 PM, Paolo Bonzini  wrote:
>
>
> On 09/10/2016 21:50, Emil Condrea wrote:
>> On Tue, Oct 4, 2016 at 11:06 AM, Paolo Bonzini  wrote:
>>>
>>>
>>> On 04/10/2016 08:43, Emil Condrea wrote:
 xen_be_frontend_changed -> xen_fe_frontend_changed
>>>
>>> This is not correct.  The front-end is implemented in the guest domain,
>>> while the back-end is implemented in the dom0 or stubdom.
>>>
>>
>> You are right, thanks for the feedback! I will drop this patch
>> together with the hunk
>> from 04/15 patch which moves this function to xen_frontend.c
>
> Actually all of your new xen_frontend.c seems to be reading frontend
> information from XenStore, which is typically something that the backend
> does.  So I suggest dropping the patch altogether.
>
> Thanks,
>
> Paolo
>
>>> This function processes *in the backed* a notification that the frontend
>>> state changed, hence the name should be xen_be_frontend_changed.
>>>
>>> Paolo
>>>
 Signed-off-by: Emil Condrea 
 ---
  hw/xen/xen_backend.c  | 2 +-
  hw/xen/xen_frontend.c | 4 ++--
  include/hw/xen/xen_frontend.h | 2 +-
  3 files changed, 4 insertions(+), 4 deletions(-)

 diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
 index 30d3aaa..b79e83e 100644
 --- a/hw/xen/xen_backend.c
 +++ b/hw/xen/xen_backend.c
 @@ -213,7 +213,7 @@ static int xen_be_try_setup(struct XenDevice *xendev)
  xen_be_set_state(xendev, XenbusStateInitialising);

  xen_be_backend_changed(xendev, NULL);
 -xen_be_frontend_changed(xendev, NULL);
 +xen_fe_frontend_changed(xendev, NULL);
  return 0;
  }

 diff --git a/hw/xen/xen_frontend.c b/hw/xen/xen_frontend.c
 index 1407f5f..761688b 100644
 --- a/hw/xen/xen_frontend.c
 +++ b/hw/xen/xen_frontend.c
 @@ -39,7 +39,7 @@ int xenstore_read_fe_uint64(struct XenDevice *xendev, 
 const char *node,
  return xenstore_read_uint64(xendev->fe, node, uval);
  }

 -void xen_be_frontend_changed(struct XenDevice *xendev, const char *node)
 +void xen_fe_frontend_changed(struct XenDevice *xendev, const char *node)
  {
  int fe_state;

 @@ -85,6 +85,6 @@ void xenstore_update_fe(char *watch, struct XenDevice 
 *xendev)
  }
  node = watch + len + 1;

 -xen_be_frontend_changed(xendev, node);
 +xen_fe_frontend_changed(xendev, node);
  xen_be_check_state(xendev);
  }
 diff --git a/include/hw/xen/xen_frontend.h b/include/hw/xen/xen_frontend.h
 index bb0bc23..2a5f03f 100644
 --- a/include/hw/xen/xen_frontend.h
 +++ b/include/hw/xen/xen_frontend.h
 @@ -9,6 +9,6 @@ int xenstore_read_fe_uint64(struct XenDevice *xendev, 
 const char *node,
  uint64_t *uval);
  void xenstore_update_fe(char *watch, struct XenDevice *xendev);

 -void xen_be_frontend_changed(struct XenDevice *xendev, const char *node);
 +void xen_fe_frontend_changed(struct XenDevice *xendev, const char *node);

  #endif /* QEMU_HW_XEN_FRONTEND_H */

>>
>>

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

Re: [Xen-devel] [PATCH 15/15] xen: Rename xen_be_frontend_changed

[Paolo Bonzini]

On 09/10/2016 21:50, Emil Condrea wrote:
> On Tue, Oct 4, 2016 at 11:06 AM, Paolo Bonzini  wrote:
>>
>>
>> On 04/10/2016 08:43, Emil Condrea wrote:
>>> xen_be_frontend_changed -> xen_fe_frontend_changed
>>
>> This is not correct.  The front-end is implemented in the guest domain,
>> while the back-end is implemented in the dom0 or stubdom.
>>
> 
> You are right, thanks for the feedback! I will drop this patch
> together with the hunk
> from 04/15 patch which moves this function to xen_frontend.c

Actually all of your new xen_frontend.c seems to be reading frontend
information from XenStore, which is typically something that the backend
does.  So I suggest dropping the patch altogether.

Thanks,

Paolo

>> This function processes *in the backed* a notification that the frontend
>> state changed, hence the name should be xen_be_frontend_changed.
>>
>> Paolo
>>
>>> Signed-off-by: Emil Condrea 
>>> ---
>>>  hw/xen/xen_backend.c  | 2 +-
>>>  hw/xen/xen_frontend.c | 4 ++--
>>>  include/hw/xen/xen_frontend.h | 2 +-
>>>  3 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
>>> index 30d3aaa..b79e83e 100644
>>> --- a/hw/xen/xen_backend.c
>>> +++ b/hw/xen/xen_backend.c
>>> @@ -213,7 +213,7 @@ static int xen_be_try_setup(struct XenDevice *xendev)
>>>  xen_be_set_state(xendev, XenbusStateInitialising);
>>>
>>>  xen_be_backend_changed(xendev, NULL);
>>> -xen_be_frontend_changed(xendev, NULL);
>>> +xen_fe_frontend_changed(xendev, NULL);
>>>  return 0;
>>>  }
>>>
>>> diff --git a/hw/xen/xen_frontend.c b/hw/xen/xen_frontend.c
>>> index 1407f5f..761688b 100644
>>> --- a/hw/xen/xen_frontend.c
>>> +++ b/hw/xen/xen_frontend.c
>>> @@ -39,7 +39,7 @@ int xenstore_read_fe_uint64(struct XenDevice *xendev, 
>>> const char *node,
>>>  return xenstore_read_uint64(xendev->fe, node, uval);
>>>  }
>>>
>>> -void xen_be_frontend_changed(struct XenDevice *xendev, const char *node)
>>> +void xen_fe_frontend_changed(struct XenDevice *xendev, const char *node)
>>>  {
>>>  int fe_state;
>>>
>>> @@ -85,6 +85,6 @@ void xenstore_update_fe(char *watch, struct XenDevice 
>>> *xendev)
>>>  }
>>>  node = watch + len + 1;
>>>
>>> -xen_be_frontend_changed(xendev, node);
>>> +xen_fe_frontend_changed(xendev, node);
>>>  xen_be_check_state(xendev);
>>>  }
>>> diff --git a/include/hw/xen/xen_frontend.h b/include/hw/xen/xen_frontend.h
>>> index bb0bc23..2a5f03f 100644
>>> --- a/include/hw/xen/xen_frontend.h
>>> +++ b/include/hw/xen/xen_frontend.h
>>> @@ -9,6 +9,6 @@ int xenstore_read_fe_uint64(struct XenDevice *xendev, const 
>>> char *node,
>>>  uint64_t *uval);
>>>  void xenstore_update_fe(char *watch, struct XenDevice *xendev);
>>>
>>> -void xen_be_frontend_changed(struct XenDevice *xendev, const char *node);
>>> +void xen_fe_frontend_changed(struct XenDevice *xendev, const char *node);
>>>
>>>  #endif /* QEMU_HW_XEN_FRONTEND_H */
>>>
> 
> 

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

Re: [Xen-devel] [PATCH 15/15] xen: Rename xen_be_frontend_changed

[Emil Condrea]

On Tue, Oct 4, 2016 at 11:06 AM, Paolo Bonzini  wrote:
>
>
> On 04/10/2016 08:43, Emil Condrea wrote:
>> xen_be_frontend_changed -> xen_fe_frontend_changed
>
> This is not correct.  The front-end is implemented in the guest domain,
> while the back-end is implemented in the dom0 or stubdom.
>

You are right, thanks for the feedback! I will drop this patch
together with the hunk
from 04/15 patch which moves this function to xen_frontend.c

> This function processes *in the backed* a notification that the frontend
> state changed, hence the name should be xen_be_frontend_changed.
>
> Paolo
>
>> Signed-off-by: Emil Condrea 
>> ---
>>  hw/xen/xen_backend.c  | 2 +-
>>  hw/xen/xen_frontend.c | 4 ++--
>>  include/hw/xen/xen_frontend.h | 2 +-
>>  3 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
>> index 30d3aaa..b79e83e 100644
>> --- a/hw/xen/xen_backend.c
>> +++ b/hw/xen/xen_backend.c
>> @@ -213,7 +213,7 @@ static int xen_be_try_setup(struct XenDevice *xendev)
>>  xen_be_set_state(xendev, XenbusStateInitialising);
>>
>>  xen_be_backend_changed(xendev, NULL);
>> -xen_be_frontend_changed(xendev, NULL);
>> +xen_fe_frontend_changed(xendev, NULL);
>>  return 0;
>>  }
>>
>> diff --git a/hw/xen/xen_frontend.c b/hw/xen/xen_frontend.c
>> index 1407f5f..761688b 100644
>> --- a/hw/xen/xen_frontend.c
>> +++ b/hw/xen/xen_frontend.c
>> @@ -39,7 +39,7 @@ int xenstore_read_fe_uint64(struct XenDevice *xendev, 
>> const char *node,
>>  return xenstore_read_uint64(xendev->fe, node, uval);
>>  }
>>
>> -void xen_be_frontend_changed(struct XenDevice *xendev, const char *node)
>> +void xen_fe_frontend_changed(struct XenDevice *xendev, const char *node)
>>  {
>>  int fe_state;
>>
>> @@ -85,6 +85,6 @@ void xenstore_update_fe(char *watch, struct XenDevice 
>> *xendev)
>>  }
>>  node = watch + len + 1;
>>
>> -xen_be_frontend_changed(xendev, node);
>> +xen_fe_frontend_changed(xendev, node);
>>  xen_be_check_state(xendev);
>>  }
>> diff --git a/include/hw/xen/xen_frontend.h b/include/hw/xen/xen_frontend.h
>> index bb0bc23..2a5f03f 100644
>> --- a/include/hw/xen/xen_frontend.h
>> +++ b/include/hw/xen/xen_frontend.h
>> @@ -9,6 +9,6 @@ int xenstore_read_fe_uint64(struct XenDevice *xendev, const 
>> char *node,
>>  uint64_t *uval);
>>  void xenstore_update_fe(char *watch, struct XenDevice *xendev);
>>
>> -void xen_be_frontend_changed(struct XenDevice *xendev, const char *node);
>> +void xen_fe_frontend_changed(struct XenDevice *xendev, const char *node);
>>
>>  #endif /* QEMU_HW_XEN_FRONTEND_H */
>>

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

Re: [Xen-devel] [PATCH 15/15] xen: Rename xen_be_frontend_changed

[Paolo Bonzini]

On 04/10/2016 10:06, Paolo Bonzini wrote:
> 
> 
> On 04/10/2016 08:43, Emil Condrea wrote:
>> xen_be_frontend_changed -> xen_fe_frontend_changed
> 
> This is not correct.  The front-end is implemented in the guest domain,
> while the back-end is implemented in the dom0 or stubdom.
> 
> This function processes *in the backed* a notification that the frontend
> state changed, hence the name should be xen_be_frontend_changed.

Sorry, this comment was in the wrong place.  It should have referred
only to the hunk in hw/xen/xen_backend.c.

Paolo

> Paolo
> 
>> Signed-off-by: Emil Condrea 
>> ---
>>  hw/xen/xen_backend.c  | 2 +-
>>  hw/xen/xen_frontend.c | 4 ++--
>>  include/hw/xen/xen_frontend.h | 2 +-
>>  3 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
>> index 30d3aaa..b79e83e 100644
>> --- a/hw/xen/xen_backend.c
>> +++ b/hw/xen/xen_backend.c
>> @@ -213,7 +213,7 @@ static int xen_be_try_setup(struct XenDevice *xendev)
>>  xen_be_set_state(xendev, XenbusStateInitialising);
>>  
>>  xen_be_backend_changed(xendev, NULL);
>> -xen_be_frontend_changed(xendev, NULL);
>> +xen_fe_frontend_changed(xendev, NULL);
>>  return 0;
>>  }
>>  
>> diff --git a/hw/xen/xen_frontend.c b/hw/xen/xen_frontend.c
>> index 1407f5f..761688b 100644
>> --- a/hw/xen/xen_frontend.c
>> +++ b/hw/xen/xen_frontend.c
>> @@ -39,7 +39,7 @@ int xenstore_read_fe_uint64(struct XenDevice *xendev, 
>> const char *node,
>>  return xenstore_read_uint64(xendev->fe, node, uval);
>>  }
>>  
>> -void xen_be_frontend_changed(struct XenDevice *xendev, const char *node)
>> +void xen_fe_frontend_changed(struct XenDevice *xendev, const char *node)
>>  {
>>  int fe_state;
>>  
>> @@ -85,6 +85,6 @@ void xenstore_update_fe(char *watch, struct XenDevice 
>> *xendev)
>>  }
>>  node = watch + len + 1;
>>  
>> -xen_be_frontend_changed(xendev, node);
>> +xen_fe_frontend_changed(xendev, node);
>>  xen_be_check_state(xendev);
>>  }
>> diff --git a/include/hw/xen/xen_frontend.h b/include/hw/xen/xen_frontend.h
>> index bb0bc23..2a5f03f 100644
>> --- a/include/hw/xen/xen_frontend.h
>> +++ b/include/hw/xen/xen_frontend.h
>> @@ -9,6 +9,6 @@ int xenstore_read_fe_uint64(struct XenDevice *xendev, const 
>> char *node,
>>  uint64_t *uval);
>>  void xenstore_update_fe(char *watch, struct XenDevice *xendev);
>>  
>> -void xen_be_frontend_changed(struct XenDevice *xendev, const char *node);
>> +void xen_fe_frontend_changed(struct XenDevice *xendev, const char *node);
>>  
>>  #endif /* QEMU_HW_XEN_FRONTEND_H */
>>
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
> 

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

Re: [Xen-devel] [PATCH 15/15] xen: Rename xen_be_frontend_changed

[Paolo Bonzini]

On 04/10/2016 08:43, Emil Condrea wrote:
> xen_be_frontend_changed -> xen_fe_frontend_changed

This is not correct.  The front-end is implemented in the guest domain,
while the back-end is implemented in the dom0 or stubdom.

This function processes *in the backed* a notification that the frontend
state changed, hence the name should be xen_be_frontend_changed.

Paolo

> Signed-off-by: Emil Condrea 
> ---
>  hw/xen/xen_backend.c  | 2 +-
>  hw/xen/xen_frontend.c | 4 ++--
>  include/hw/xen/xen_frontend.h | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
> index 30d3aaa..b79e83e 100644
> --- a/hw/xen/xen_backend.c
> +++ b/hw/xen/xen_backend.c
> @@ -213,7 +213,7 @@ static int xen_be_try_setup(struct XenDevice *xendev)
>  xen_be_set_state(xendev, XenbusStateInitialising);
>  
>  xen_be_backend_changed(xendev, NULL);
> -xen_be_frontend_changed(xendev, NULL);
> +xen_fe_frontend_changed(xendev, NULL);
>  return 0;
>  }
>  
> diff --git a/hw/xen/xen_frontend.c b/hw/xen/xen_frontend.c
> index 1407f5f..761688b 100644
> --- a/hw/xen/xen_frontend.c
> +++ b/hw/xen/xen_frontend.c
> @@ -39,7 +39,7 @@ int xenstore_read_fe_uint64(struct XenDevice *xendev, const 
> char *node,
>  return xenstore_read_uint64(xendev->fe, node, uval);
>  }
>  
> -void xen_be_frontend_changed(struct XenDevice *xendev, const char *node)
> +void xen_fe_frontend_changed(struct XenDevice *xendev, const char *node)
>  {
>  int fe_state;
>  
> @@ -85,6 +85,6 @@ void xenstore_update_fe(char *watch, struct XenDevice 
> *xendev)
>  }
>  node = watch + len + 1;
>  
> -xen_be_frontend_changed(xendev, node);
> +xen_fe_frontend_changed(xendev, node);
>  xen_be_check_state(xendev);
>  }
> diff --git a/include/hw/xen/xen_frontend.h b/include/hw/xen/xen_frontend.h
> index bb0bc23..2a5f03f 100644
> --- a/include/hw/xen/xen_frontend.h
> +++ b/include/hw/xen/xen_frontend.h
> @@ -9,6 +9,6 @@ int xenstore_read_fe_uint64(struct XenDevice *xendev, const 
> char *node,
>  uint64_t *uval);
>  void xenstore_update_fe(char *watch, struct XenDevice *xendev);
>  
> -void xen_be_frontend_changed(struct XenDevice *xendev, const char *node);
> +void xen_fe_frontend_changed(struct XenDevice *xendev, const char *node);
>  
>  #endif /* QEMU_HW_XEN_FRONTEND_H */
> 

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