RE: DOM0_GETDOMAININFO intended behavior

RE: DOM0_GETDOMAININFO intended behavior

[Ian Pratt]

 
> You're right, it isn't difficult to check what is returned - 
> provided you know what to expect. If you're new, however, to 
> the API and unfamiliar with the fact that it returns the next 
> domain in the line, you could hit what we just hit. We built 
> an application to use the interface and our tests returned 
> some odd behavior. We had to delve deeper to find out if this 
> was correct or not. 

You're not the first person to be caught out by this -- I seem to recall
that xenctx forgets to do the domid check too.

Passing a flag in to explicitly request that you want to iterate would
probably be an improvement to the interface.

Ian

> The current behavior is just not obvious. Perhaps just 
> documenting it in the code and the manual is enough so others 
> don't hit this. 
> 
> Thanks,
> 
> Dan
> 

Re: DOM0_GETDOMAININFO intended behavior

[Kip Macy]

> You're not the first person to be caught out by this -- I seem to recall
> that xenctx forgets to do the domid check too.
> 
> Passing a flag in to explicitly request that you want to iterate would
> probably be an improvement to the interface.

I agree. That way the default behaviour would be what most people expect.

                      -Kip


> 
> Ian
> 
> > The current behavior is just not obvious. Perhaps just
> > documenting it in the code and the manual is enough so others
> > don't hit this.
> >
> > Thanks,
> >
> > Dan
> >
>

RE: DOM0_GETDOMAININFO intended behavior

[Daniel Stekloff]

[-- Attachment #1: Type: text/plain, Size: 1019 bytes --]

On Sat, 2005-06-04 at 22:51 +0100, Ian Pratt wrote:
>  > You're right, it isn't difficult to check what is returned - 
> > provided you know what to expect. If you're new, however, to 
> > the API and unfamiliar with the fact that it returns the next 
> > domain in the line, you could hit what we just hit. We built 
> > an application to use the interface and our tests returned 
> > some odd behavior. We had to delve deeper to find out if this 
> > was correct or not. 
> 
> You're not the first person to be caught out by this -- I seem to recall
> that xenctx forgets to do the domid check too.
> 
> Passing a flag in to explicitly request that you want to iterate would
> probably be an improvement to the interface.



Instead of changing the interface and the applications, how about the
following: If you request a specific domain id and it doesn't exist, you
get back the expected result. If, however, you want a list of domains it
works like it has been.

Thanks,

Dan

Signed-off-by: dsteklof@us.ibm.com



[-- Attachment #2: xc_domain_getinfo.patch --]
[-- Type: text/x-patch, Size: 591 bytes --]

--- xeno-unstable-latest/tools/libxc/xc_domain.c	2005-06-06 11:53:03.000000000 -0700
+++ xeno-unstable/tools/libxc/xc_domain.c	2005-06-06 15:28:59.000000000 -0700
@@ -83,6 +83,11 @@
         op.u.getdomaininfo.domain = (domid_t)next_domid;
         if ( (rc = do_dom0_op(xc_handle, &op)) < 0 )
             break;
+        if ( max_doms == 1 && op.u.getdomaininfo.domain != first_domid )
+        {
+            rc = -ESRCH;
+            break;
+        }
         info->domid      = (u16)op.u.getdomaininfo.domain;
 
         info->dying    = !!(op.u.getdomaininfo.flags & DOMFLAGS_DYING);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

RE: DOM0_GETDOMAININFO intended behavior

[Macy, Kip]

> I think the cleanest fix is just to add an 'iterate' flag to the 
> parameters in GETDOMAININFO, no?


Ok... now that I (unintentionally) went overboard on this, why not just
document how it works and leave it as is? What will the flag really buy
you? 

Behaviour consistent with the other DOM0 operations.

-Kip

RE: DOM0_GETDOMAININFO intended behavior

[Daniel Stekloff]

[-- Attachment #1: Type: text/plain, Size: 1108 bytes --]

On Wed, 2005-06-08 at 11:42 +0100, Ian Pratt wrote:
> > > Passing a flag in to explicitly request that you want to 
> > iterate would 
> > > probably be an improvement to the interface. 
> > 
> > Instead of changing the interface and the applications, how about the
> > following: If you request a specific domain id and it doesn't 
> > exist, you get back the expected result. If, however, you 
> > want a list of domains it works like it has been.
> 
> Hmm, I'm not sure I like this. It turns a relatively minor 'gotcha' of
> the hypervisor interface into something that's more confusing: if you're
> trying to iterate over domains using xc_domain_getinfo then you have to
> ask for at least two domains at a time otherwise you'll get ESRCH.
> 
> I think the cleanest fix is just to add an 'iterate' flag to the
> parameters in GETDOMAININFO, no?


Ok... now that I (unintentionally) went overboard on this, why not just
document how it works and leave it as is? What will the flag really buy
you? 

Sorry for the hoo ha. <grin>

What about just adding the following:

signed-off-by: dsteklof@us.ibm.com



[-- Attachment #2: xc_domain_getinfo-add-comment.patch --]
[-- Type: text/x-patch, Size: 799 bytes --]

--- xeno-unstable-latest/tools/libxc/xc.h	2005-06-09 15:32:01.000000000 -0700
+++ xeno-unstable/tools/libxc/xc.h	2005-06-09 15:29:02.000000000 -0700
@@ -169,7 +169,11 @@
                      int vcpu,
                      cpumap_t *cpumap);
 /**
- * This function will return information about one or more domains.
+ * This function will return information about one or more domains. It is
+ * designed to iterate over the list of domains. If a single domain is
+ * requested, this function will return the next domain in the list - if
+ * one exists. It is, therefore, important in this case to make sure the
+ * domain requested was the one returned.
  *
  * @parm xc_handle a handle to an open hypervisor interface
  * @parm first_domid the first domain to enumerate information from.  Domains

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

RE: DOM0_GETDOMAININFO intended behavior

[Ian Pratt]

> > Passing a flag in to explicitly request that you want to 
> iterate would 
> > probably be an improvement to the interface. 
> 
> Instead of changing the interface and the applications, how about the
> following: If you request a specific domain id and it doesn't 
> exist, you get back the expected result. If, however, you 
> want a list of domains it works like it has been.

Hmm, I'm not sure I like this. It turns a relatively minor 'gotcha' of
the hypervisor interface into something that's more confusing: if you're
trying to iterate over domains using xc_domain_getinfo then you have to
ask for at least two domains at a time otherwise you'll get ESRCH.

I think the cleanest fix is just to add an 'iterate' flag to the
parameters in GETDOMAININFO, no?

Ian

Re: DOM0_GETDOMAININFO intended behavior

[Daniel Stekloff]

On Saturday 04 June 2005 12:42, Christian Limpach wrote:
> On 6/4/05, Daniel Stekloff <dsteklof@us.ibm.com> wrote:
> > On Saturday 04 June 2005 01:20, Ian Pratt wrote:
> > > > :-)
> > > >
> > > > I asked the same question a week or two ago and yes it is
> > > > intended. It provides for easy iteration. I think it is kind
> > > > of a hackish interface as it isn't uniform with *all* the
> > > > other DOM0 calls. However, it works and once you know to
> > > > check the domid return value your code will work as expected.
> > >
> > > I suppose we could add a flag to indicate whether you were iterating or
> > > wanted information on a specific domain. I think currently the only
> > > case where we want to iterate is 'xm list'.
> >
> > If you're ok with changing the interface, wouldn't it be better for the
> > hypercall to get and return just the requested domain info while you move
> > the interation to libxc? The library could have two calls, one which
> > calls the other: xc_domain_getinfo() and xc_domain_getinfo_list()
>
> This doesn't scale well if your domain id space is sparse, which it
> can easily become on a server which has many domains restarting
> frequently.  This is the reason for the current behaviour.
>
> I don't see what's wrong with the current behaviour anyway, how hard
> is it to check if the call succeeded and then additionally check if
> information for the requested domain was returned?  We could avoid the
> additional check by having a different return code for when the call
> succeeded but returned information for a different domain than the
> requested one.


Sorry... I didn't mean to make a big deal of this... I just needed to make 
sure if it was intended. Now I can go an patch vm-list to deal with this 
correctly.

You're right, it isn't difficult to check what is returned - provided you know 
what to expect. If you're new, however, to the API and unfamiliar with the 
fact that it returns the next domain in the line, you could hit what we just 
hit. We built an application to use the interface and our tests returned some 
odd behavior. We had to delve deeper to find out if this was correct or not. 
This led us to query you. 

The current behavior is just not obvious. Perhaps just documenting it in the 
code and the manual is enough so others don't hit this. 

Thanks,

Dan

Re: DOM0_GETDOMAININFO intended behavior

[Christian Limpach]

On 6/4/05, Daniel Stekloff <dsteklof@us.ibm.com> wrote:
> On Saturday 04 June 2005 01:20, Ian Pratt wrote:
> > > :-)
> > >
> > > I asked the same question a week or two ago and yes it is
> > > intended. It provides for easy iteration. I think it is kind
> > > of a hackish interface as it isn't uniform with *all* the
> > > other DOM0 calls. However, it works and once you know to
> > > check the domid return value your code will work as expected.
> >
> > I suppose we could add a flag to indicate whether you were iterating or
> > wanted information on a specific domain. I think currently the only case
> > where we want to iterate is 'xm list'.
> 
> 
> If you're ok with changing the interface, wouldn't it be better for the
> hypercall to get and return just the requested domain info while you move the
> interation to libxc? The library could have two calls, one which calls the
> other: xc_domain_getinfo() and xc_domain_getinfo_list()

This doesn't scale well if your domain id space is sparse, which it
can easily become on a server which has many domains restarting
frequently.  This is the reason for the current behaviour.

I don't see what's wrong with the current behaviour anyway, how hard
is it to check if the call succeeded and then additionally check if
information for the requested domain was returned?  We could avoid the
additional check by having a different return code for when the call
succeeded but returned information for a different domain than the
requested one.

    christian

Re: DOM0_GETDOMAININFO intended behavior

[Daniel Stekloff]

On Saturday 04 June 2005 01:20, Ian Pratt wrote:
> > :-)
> >
> > I asked the same question a week or two ago and yes it is
> > intended. It provides for easy iteration. I think it is kind
> > of a hackish interface as it isn't uniform with *all* the
> > other DOM0 calls. However, it works and once you know to
> > check the domid return value your code will work as expected.
>
> I suppose we could add a flag to indicate whether you were iterating or
> wanted information on a specific domain. I think currently the only case
> where we want to iterate is 'xm list'.


If you're ok with changing the interface, wouldn't it be better for the 
hypercall to get and return just the requested domain info while you move the 
interation to libxc? The library could have two calls, one which calls the 
other: xc_domain_getinfo() and xc_domain_getinfo_list()

This way you'd have a hypercall that returns expected information and you 
wouldn't have to worry about flags. 

Thanks,

Dan

RE: DOM0_GETDOMAININFO intended behavior

[Ian Pratt]

 
> :-)
> I asked the same question a week or two ago and yes it is 
> intended. It provides for easy iteration. I think it is kind 
> of a hackish interface as it isn't uniform with *all* the 
> other DOM0 calls. However, it works and once you know to 
> check the domid return value your code will work as expected.

I suppose we could add a flag to indicate whether you were iterating or
wanted information on a specific domain. I think currently the only case
where we want to iterate is 'xm list'.

Ian

Re: DOM0_GETDOMAININFO intended behavior

[Anthony Liguori]

Kip Macy wrote:

>:-) 
>I asked the same question a week or two ago and yes it is intended. It
>provides for easy iteration. I think it is kind of a hackish interface
>as it isn't uniform with *all* the other DOM0 calls. However, it works
>and once you know to check the domid return value your code will work
>as expected.
>  
>
It's always concerned me that something there isn't an atomic interface 
for getting all running domain information.  I've not been able to 
figure out a situation though where it would create a problem.

It just seems kind of odd that you could do an xc_domain_getinfo of an 
array of dominfo's and a portion of the array might not be valid when 
other portions of the array are.  It's possible that the call could 
return two domains that never actually existed together at the same 
moment in time.

Again, I don't think it's a race condition but it's just odd.

Regards,

Anthony Liguori

>                      -Kip
>
>On 6/3/05, Daniel Stekloff <dsteklof@us.ibm.com> wrote:
>  
>
>>Hi,
>>
>>Is it intended behavior for DOM0_GETDOMAININFO to return the next
>>domain's info if a requested domain doesn't exist?
>>
>>In xeno-unstable - xen/common/dom0_ops.c - lines 310-325:
>>
>>        for_each_domain ( d )
>>        {
>>            if ( d->domain_id >= op->u.getdomaininfo.domain )
>>                break;
>>        }
>>
>>        if ( (d == NULL) || !get_domain(d) )
>>        {
>>            read_unlock(&domlist_lock);
>>            ret = -ESRCH;
>>            break;
>>        }
>>
>>        read_unlock(&domlist_lock);
>>
>>        op->u.getdomaininfo.domain = d->domain_id;
>>
>>
>>
>>If, as an example, I request info for domain 2 that doesn't exist
>>anymore and a higher domain number does exist, xen will return the next
>>domain's information rather than an error telling me domain 2 doesn't
>>exist.
>>
>>Is this correct?
>>
>>I noticed that libxc's xc_domain_getinfo() is built to use this when
>>grabbing multiple domain information. I want to know if we need to fix
>>vm-list to check what's returned or if this is unwanted behavior in the
>>library and hypervisor.
>>
>>Thanks,
>>
>>Dan
>>
>>
>>
>>
>>
>>
>>
>>_______________________________________________
>>Xen-devel mailing list
>>Xen-devel@lists.xensource.com
>>http://lists.xensource.com/xen-devel
>>
>>    
>>
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@lists.xensource.com
>http://lists.xensource.com/xen-devel
>
>  
>

Re: DOM0_GETDOMAININFO intended behavior

[Kip Macy]

:-) 
I asked the same question a week or two ago and yes it is intended. It
provides for easy iteration. I think it is kind of a hackish interface
as it isn't uniform with *all* the other DOM0 calls. However, it works
and once you know to check the domid return value your code will work
as expected.


                      -Kip

On 6/3/05, Daniel Stekloff <dsteklof@us.ibm.com> wrote:
> 
> Hi,
> 
> Is it intended behavior for DOM0_GETDOMAININFO to return the next
> domain's info if a requested domain doesn't exist?
> 
> In xeno-unstable - xen/common/dom0_ops.c - lines 310-325:
> 
>         for_each_domain ( d )
>         {
>             if ( d->domain_id >= op->u.getdomaininfo.domain )
>                 break;
>         }
> 
>         if ( (d == NULL) || !get_domain(d) )
>         {
>             read_unlock(&domlist_lock);
>             ret = -ESRCH;
>             break;
>         }
> 
>         read_unlock(&domlist_lock);
> 
>         op->u.getdomaininfo.domain = d->domain_id;
> 
> 
> 
> If, as an example, I request info for domain 2 that doesn't exist
> anymore and a higher domain number does exist, xen will return the next
> domain's information rather than an error telling me domain 2 doesn't
> exist.
> 
> Is this correct?
> 
> I noticed that libxc's xc_domain_getinfo() is built to use this when
> grabbing multiple domain information. I want to know if we need to fix
> vm-list to check what's returned or if this is unwanted behavior in the
> library and hypervisor.
> 
> Thanks,
> 
> Dan
> 
> 
> 
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

DOM0_GETDOMAININFO intended behavior

[Daniel Stekloff]

Hi,

Is it intended behavior for DOM0_GETDOMAININFO to return the next
domain's info if a requested domain doesn't exist? 

In xeno-unstable - xen/common/dom0_ops.c - lines 310-325:

        for_each_domain ( d )
        {
            if ( d->domain_id >= op->u.getdomaininfo.domain )
                break;
        }

        if ( (d == NULL) || !get_domain(d) )
        {
            read_unlock(&domlist_lock);
            ret = -ESRCH;
            break;
        }

        read_unlock(&domlist_lock);

        op->u.getdomaininfo.domain = d->domain_id;



If, as an example, I request info for domain 2 that doesn't exist
anymore and a higher domain number does exist, xen will return the next
domain's information rather than an error telling me domain 2 doesn't
exist.

Is this correct? 

I noticed that libxc's xc_domain_getinfo() is built to use this when
grabbing multiple domain information. I want to know if we need to fix
vm-list to check what's returned or if this is unwanted behavior in the
library and hypervisor.

Thanks,

Dan