tools/xenstore: set maximum number of grants needed

[PATCH] tools/xenstore: set maximum number of grants needed

Posted by Juergen Gross 4 years, 2 months ago

When running as a stubdom Xenstore should set the maximum number of
grants needed via a call of xengnttab_set_max_grants(), as otherwise
the number of domains which can be supported will be 128 only (the
default number of grants supported by Mini-OS).

Signed-off-by: Juergen Gross <jgross@suse.com>
---
This is a backport candidate IMO.
---
 tools/xenstore/xenstored_domain.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index 06359503f0..f740aa02f5 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -630,6 +630,8 @@ void domain_init(void)
 	*xgt_handle = xengnttab_open(NULL, 0);
 	if (*xgt_handle == NULL)
 		barf_perror("Failed to open connection to gnttab");
+	/* Allow max number of domains for mappings. */
+	xengnttab_set_max_grants(*xgt_handle, DOMID_FIRST_RESERVED);
 
 	talloc_set_destructor(xgt_handle, close_xgt_handle);
 
-- 
2.26.2

Re: [PATCH] tools/xenstore: set maximum number of grants needed

Posted by Wei Liu 4 years, 2 months ago

On Thu, Sep 24, 2020 at 04:36:48PM +0200, Juergen Gross wrote:
> When running as a stubdom Xenstore should set the maximum number of
> grants needed via a call of xengnttab_set_max_grants(), as otherwise
> the number of domains which can be supported will be 128 only (the
> default number of grants supported by Mini-OS).
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> This is a backport candidate IMO.
> ---
>  tools/xenstore/xenstored_domain.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
> index 06359503f0..f740aa02f5 100644
> --- a/tools/xenstore/xenstored_domain.c
> +++ b/tools/xenstore/xenstored_domain.c
> @@ -630,6 +630,8 @@ void domain_init(void)
>  	*xgt_handle = xengnttab_open(NULL, 0);
>  	if (*xgt_handle == NULL)
>  		barf_perror("Failed to open connection to gnttab");
> +	/* Allow max number of domains for mappings. */
> +	xengnttab_set_max_grants(*xgt_handle, DOMID_FIRST_RESERVED);

Why DOMID_FIRST_RESERVED as the count argument? Is the expectation here
xenstored maps one grant per domain?

Wei.

>  
>  	talloc_set_destructor(xgt_handle, close_xgt_handle);
>  
> -- 
> 2.26.2
>

Re: [PATCH] tools/xenstore: set maximum number of grants needed

Posted by Jürgen Groß 4 years, 2 months ago

On 30.09.20 17:14, Wei Liu wrote:
> On Thu, Sep 24, 2020 at 04:36:48PM +0200, Juergen Gross wrote:
>> When running as a stubdom Xenstore should set the maximum number of
>> grants needed via a call of xengnttab_set_max_grants(), as otherwise
>> the number of domains which can be supported will be 128 only (the
>> default number of grants supported by Mini-OS).
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> This is a backport candidate IMO.
>> ---
>>   tools/xenstore/xenstored_domain.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
>> index 06359503f0..f740aa02f5 100644
>> --- a/tools/xenstore/xenstored_domain.c
>> +++ b/tools/xenstore/xenstored_domain.c
>> @@ -630,6 +630,8 @@ void domain_init(void)
>>   	*xgt_handle = xengnttab_open(NULL, 0);
>>   	if (*xgt_handle == NULL)
>>   		barf_perror("Failed to open connection to gnttab");
>> +	/* Allow max number of domains for mappings. */
>> +	xengnttab_set_max_grants(*xgt_handle, DOMID_FIRST_RESERVED);
> 
> Why DOMID_FIRST_RESERVED as the count argument? Is the expectation here
> xenstored maps one grant per domain?

Yes. This is the theoretical maximum.


Juergen

Re: [PATCH] tools/xenstore: set maximum number of grants needed

Posted by Wei Liu 4 years, 2 months ago

On Wed, Sep 30, 2020 at 05:23:58PM +0200, Jürgen Groß wrote:
> On 30.09.20 17:14, Wei Liu wrote:
> > On Thu, Sep 24, 2020 at 04:36:48PM +0200, Juergen Gross wrote:
> > > When running as a stubdom Xenstore should set the maximum number of
> > > grants needed via a call of xengnttab_set_max_grants(), as otherwise
> > > the number of domains which can be supported will be 128 only (the
> > > default number of grants supported by Mini-OS).
> > > 
> > > Signed-off-by: Juergen Gross <jgross@suse.com>
> > > ---
> > > This is a backport candidate IMO.
> > > ---
> > >   tools/xenstore/xenstored_domain.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
> > > index 06359503f0..f740aa02f5 100644
> > > --- a/tools/xenstore/xenstored_domain.c
> > > +++ b/tools/xenstore/xenstored_domain.c
> > > @@ -630,6 +630,8 @@ void domain_init(void)
> > >   	*xgt_handle = xengnttab_open(NULL, 0);
> > >   	if (*xgt_handle == NULL)
> > >   		barf_perror("Failed to open connection to gnttab");
> > > +	/* Allow max number of domains for mappings. */
> > > +	xengnttab_set_max_grants(*xgt_handle, DOMID_FIRST_RESERVED);
> > 
> > Why DOMID_FIRST_RESERVED as the count argument? Is the expectation here
> > xenstored maps one grant per domain?
> 
> Yes. This is the theoretical maximum.
> 

In that case:

Acked-by: Wei Liu <wl@xen.org>

I will also add that information to the commit message.

> 
> Juergen

Re: [PATCH] tools/xenstore: set maximum number of grants needed

Posted by Andrew Cooper 4 years, 2 months ago

On 30/09/2020 16:28, Wei Liu wrote:
> On Wed, Sep 30, 2020 at 05:23:58PM +0200, Jürgen Groß wrote:
>> On 30.09.20 17:14, Wei Liu wrote:
>>> On Thu, Sep 24, 2020 at 04:36:48PM +0200, Juergen Gross wrote:
>>>> When running as a stubdom Xenstore should set the maximum number of
>>>> grants needed via a call of xengnttab_set_max_grants(), as otherwise
>>>> the number of domains which can be supported will be 128 only (the
>>>> default number of grants supported by Mini-OS).
>>>>
>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>> ---
>>>> This is a backport candidate IMO.
>>>> ---
>>>>   tools/xenstore/xenstored_domain.c | 2 ++
>>>>   1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
>>>> index 06359503f0..f740aa02f5 100644
>>>> --- a/tools/xenstore/xenstored_domain.c
>>>> +++ b/tools/xenstore/xenstored_domain.c
>>>> @@ -630,6 +630,8 @@ void domain_init(void)
>>>>   	*xgt_handle = xengnttab_open(NULL, 0);
>>>>   	if (*xgt_handle == NULL)
>>>>   		barf_perror("Failed to open connection to gnttab");
>>>> +	/* Allow max number of domains for mappings. */
>>>> +	xengnttab_set_max_grants(*xgt_handle, DOMID_FIRST_RESERVED);
>>> Why DOMID_FIRST_RESERVED as the count argument? Is the expectation here
>>> xenstored maps one grant per domain?
>> Yes. This is the theoretical maximum.
>>
> In that case:
>
> Acked-by: Wei Liu <wl@xen.org>
>
> I will also add that information to the commit message.

And the comment if possible please.  That's where it will be most useful
to the next person doing a doubletake at this code.  :)

~Andrew

Re: [PATCH] tools/xenstore: set maximum number of grants needed

Posted by Wei Liu 4 years, 2 months ago

On Wed, Sep 30, 2020 at 04:30:16PM +0100, Andrew Cooper wrote:
> On 30/09/2020 16:28, Wei Liu wrote:
> > On Wed, Sep 30, 2020 at 05:23:58PM +0200, Jürgen Groß wrote:
> >> On 30.09.20 17:14, Wei Liu wrote:
> >>> On Thu, Sep 24, 2020 at 04:36:48PM +0200, Juergen Gross wrote:
> >>>> When running as a stubdom Xenstore should set the maximum number of
> >>>> grants needed via a call of xengnttab_set_max_grants(), as otherwise
> >>>> the number of domains which can be supported will be 128 only (the
> >>>> default number of grants supported by Mini-OS).
> >>>>
> >>>> Signed-off-by: Juergen Gross <jgross@suse.com>
> >>>> ---
> >>>> This is a backport candidate IMO.
> >>>> ---
> >>>>   tools/xenstore/xenstored_domain.c | 2 ++
> >>>>   1 file changed, 2 insertions(+)
> >>>>
> >>>> diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
> >>>> index 06359503f0..f740aa02f5 100644
> >>>> --- a/tools/xenstore/xenstored_domain.c
> >>>> +++ b/tools/xenstore/xenstored_domain.c
> >>>> @@ -630,6 +630,8 @@ void domain_init(void)
> >>>>   	*xgt_handle = xengnttab_open(NULL, 0);
> >>>>   	if (*xgt_handle == NULL)
> >>>>   		barf_perror("Failed to open connection to gnttab");
> >>>> +	/* Allow max number of domains for mappings. */
> >>>> +	xengnttab_set_max_grants(*xgt_handle, DOMID_FIRST_RESERVED);
> >>> Why DOMID_FIRST_RESERVED as the count argument? Is the expectation here
> >>> xenstored maps one grant per domain?
> >> Yes. This is the theoretical maximum.
> >>
> > In that case:
> >
> > Acked-by: Wei Liu <wl@xen.org>
> >
> > I will also add that information to the commit message.
> 
> And the comment if possible please.  That's where it will be most useful
> to the next person doing a doubletake at this code.  :)

Yes sure.

Wei.

> 
> ~Andrew