[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On 2015/5/19 19:00, Wei Liu wrote:
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?

You're 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

Definitely.

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?

I think we can set the default 'type' to NONE,

libxl__rdm_setdefault()
{
    b_info->rdm.type = LIBXL_RDM_RESERVE_TYPE_NONE;

and then,

libxl__domain_device_construct_rdm()
{
    ...
    /* Might not expose rdm. */
    if (type == LIBXL_RDM_RESERVE_TYPE_NONE)
        return 0;

This means we don't expose any rdm so we would never concern any policy anymore.


Thanks
Tiejun



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.