Re: [Xen-devel] [RFC][PATCH 01/13] tools: introduce some new parameters to set rdm policy

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, May 19, 2015 at 06:50:11PM +0800, Chen, Tiejun wrote:
> On 2015/5/19 17:42, Wei Liu wrote:

[...]

> >>>>>>>want you want?
> >>>>>>
> >>>>>>We have a little bit of complexity here,
> >>>>>>
> >>>>>>"Default per-device RDM policy is 'force', while default global RDM 
> >>>>>>policy
> >>>>>>is 'try'. When both policies are specified on a given region, 'force' is
> >>>>>>always preferred."
> >>>>>>
> >>>>>
> >>>>>This is going to be done in actual code anyway.
> >>>>>
> >>>>>This type is used both in global and per-device setting, so I envisage
> >>>>
> >>>>Yes.
> >>>>
> >>>>>this to have an invalid value to start with. Appropriate default values
> >>>>
> >>>>Sounds I should set this,
> >>>>
> >>>>+libxl_rdm_reserve_flag = Enumeration("rdm_reserve_flag", [
> >>>>+    (-1, "invalid"),
> >>>>+    (0, "strict"),
> >>>>+    (1, "relaxed"),
> >>>>+    ], init_val = "LIBXL_RDM_RESERVE_FLAG_INVALID")
> >>>>+
> >>>>
> >>>
> >
> >Yet another question about this feature. The current setup suggests that
> >we must choose a policy, either "strict" or "relaxed", i.e. there is no
> >way to disable this feature, as in there is no "none" policy to skip
> >checking rdm conflict.
> >
> >AIUI this feature is more like a bug fix to existing problem, so we
> >always want to enable it. And the default relaxed policy makes sure we
> >don't break guest that was working before this feature. Do I understand
> >this correctly?
> >
> >If we risk breaking existing guests, we might want to consider adding a
> >"none" (name subject to improvement) policy to just skip RDM all
> >together.
> 
> We have this definition,
> 
> +libxl_rdm_reserve_type = Enumeration("rdm_reserve_type", [
> +    (0, "none"),
> +    (1, "host"),
> +    ])
> 
> If we set 'type=none', this means we would do nothing actually since we
> don't expose any rdms to guest. This behavior will ensue we go back the
> existing scenario without our patch.
> 

But this only works with global configuration and individual
configuration in PCI spec trumps this, right?

Think about how an old configuration migrated to newer version of Xen
should work. For example, I don't have rdm= but have pci=['xxxx']. Do we
need to make sure this still work? I guess the answer is if it already
works before RDM it should continue to work as there is really no
conflict before. In this case whether  we enable RDM or not doesn't make
a difference to a guest that's already working before. Am I right?

> >
> >>>Yes, and then don't forget to set the value to appropriate value in the
> >>>_setdefault functions for different types.
> >>

[...]

> >Spaces.
> >
> >>>>>>>>+    if ( NULL == (buf2 = ptr = strdup(str)) )
> >>>>>>>>+        return ERROR_NOMEM;
> >>>>>>>>+
> >>>>>>>>+    for(tok = ptr, end = ptr + strlen(ptr) + 1; ptr < end; ptr++) {
> >>>>>>>>+        switch(state) {
> >>
> >>I thought initially you let me to follow that previous "if" :)
> >>
> >
> >Just be consistent with other part of the source code.
> 
> I just refer to that existing xlu_pci_parse_bdf()...
> 

Sorry I didn't mean to blame you for something that's not your fault.

> Anyway I guess you mean I should do something like this,
> 
>     if (NULL == (buf2 = ptr = strdup(str)))
>         return ERROR_NOMEM;
> 
>     for (tok = ptr, end = ptr + strlen(ptr) + 1; ptr < end; ptr++) {
>         switch(state) {
>         case STATE_TYPE:
>             if (*ptr == '=') {
>           ...
> 

Fair enough. I prefer consistency.

Wei.

> Thanks
> Tiejun
> 

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