diff mbox series

[4/6] domctl: set XEN_DOMCTL_createdomain 'rover' if valid domid is specified

Message ID 20191224130416.3570-5-pdurrant@amazon.com (mailing list archive)
State New, archived
Headers show
Series xl/libxl: allow creation of domains with a specified domid | expand

Commit Message

Paul Durrant Dec. 24, 2019, 1:04 p.m. UTC
The value of 'rover' is the value at which Xen will start searching for an
unused domid if none is specified. Currently it is only updated when a
domid is automatically chosen, rather than specified by the caller, which
makes it very hard to describe Xen's rationale in choosing domids in an
environment where some domain creations have specified domids and some
don't.
This patch always updates 'rover' after a successful creation, even in the
case that domid is specified by the caller. This ensures that, if Xen
automatically chooses a domid for a subsequent domain creation it will
always be the next available value after the domid of the most recently
created domain.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien@xen.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Wei Liu <wl@xen.org>
---
 xen/common/domctl.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Ian Jackson Jan. 2, 2020, 5:25 p.m. UTC | #1
Paul Durrant writes ("[PATCH 4/6] domctl: set XEN_DOMCTL_createdomain 'rover' if valid domid is specified"):
> The value of 'rover' is the value at which Xen will start searching for an
> unused domid if none is specified. Currently it is only updated when a
> domid is automatically chosen, rather than specified by the caller, which
> makes it very hard to describe Xen's rationale in choosing domids in an
> environment where some domain creations have specified domids and some
> don't.
> This patch always updates 'rover' after a successful creation, even in the
> case that domid is specified by the caller. This ensures that, if Xen
> automatically chooses a domid for a subsequent domain creation it will
> always be the next available value after the domid of the most recently
> created domain.

I'm not yet convinced this behaviour is better, but I'm open to
persuasion.

The existing allocation system falls down in any case if the domid
gets near to wrapping round.  If it doesn't, then without this patch
it is possible to have two ranges of domids: automatically allocated
ones, and statically allocated high ones.

With this patch, statically allocating a domid resets rover and causes
the remaining bits of static space to be polluted.

What am I missing ?  What are the use cases here ?

Thanks,
Ian.
Andrew Cooper Jan. 2, 2020, 5:30 p.m. UTC | #2
On 24/12/2019 13:04, Paul Durrant wrote:
> The value of 'rover' is the value at which Xen will start searching for an
> unused domid if none is specified. Currently it is only updated when a
> domid is automatically chosen, rather than specified by the caller, which
> makes it very hard to describe Xen's rationale in choosing domids in an
> environment where some domain creations have specified domids and some
> don't.
> This patch always updates 'rover' after a successful creation, even in the
> case that domid is specified by the caller. This ensures that, if Xen
> automatically chooses a domid for a subsequent domain creation it will
> always be the next available value after the domid of the most recently
> created domain.
>
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>

This behaviour is deliberate, if not obvious.

Lots of things break when domid's overflow.  The current behaviour means
that an overflow won't occur until all domids have been used.

Domid reuse is a massive swamp which someone is going to fix at some
point...

~Andrew
Paul Durrant Jan. 3, 2020, 9:11 a.m. UTC | #3
> -----Original Message-----
> From: Ian Jackson <ian.jackson@citrix.com>
> Sent: 02 January 2020 17:25
> To: Durrant, Paul <pdurrant@amazon.com>
> Cc: xen-devel@lists.xenproject.org; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; George Dunlap <George.Dunlap@citrix.com>; Jan
> Beulich <jbeulich@suse.com>; Julien Grall <julien@xen.org>; Konrad
> Rzeszutek Wilk <konrad.wilk@oracle.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Wei Liu <wl@xen.org>
> Subject: Re: [PATCH 4/6] domctl: set XEN_DOMCTL_createdomain 'rover' if
> valid domid is specified
> 
> Paul Durrant writes ("[PATCH 4/6] domctl: set XEN_DOMCTL_createdomain
> 'rover' if valid domid is specified"):
> > The value of 'rover' is the value at which Xen will start searching for
> an
> > unused domid if none is specified. Currently it is only updated when a
> > domid is automatically chosen, rather than specified by the caller,
> which
> > makes it very hard to describe Xen's rationale in choosing domids in an
> > environment where some domain creations have specified domids and some
> > don't.
> > This patch always updates 'rover' after a successful creation, even in
> the
> > case that domid is specified by the caller. This ensures that, if Xen
> > automatically chooses a domid for a subsequent domain creation it will
> > always be the next available value after the domid of the most recently
> > created domain.
> 
> I'm not yet convinced this behaviour is better, but I'm open to
> persuasion.
> 
> The existing allocation system falls down in any case if the domid
> gets near to wrapping round.  If it doesn't, then without this patch
> it is possible to have two ranges of domids: automatically allocated
> ones, and statically allocated high ones.
> 
> With this patch, statically allocating a domid resets rover and causes
> the remaining bits of static space to be polluted.
> 
> What am I missing ?  What are the use cases here ?

The problem I was tackling was trying to document how Xen chooses a domid. E.g. it is not correct to say that it chooses the lowest numbered available domid. The best I could come up with was 'the next available domid after the last one it thought of' which is pretty poor... and also becomes harder to explain if some domids are not actually chosen by Xen, because the toolstack specified them.

The end game which this series is working towards is transparent migration which, because of the 'design' of PV protocols and certain hypercalls in the guest ABI, requires that the domid is persisted on migration. This patch is simply trying to lay groundwork for the co-existence of domains with specified domids and those with automatically allocated domids. 

  Paul

> 
> Thanks,
> Ian.
diff mbox series

Patch

diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 650310e874..5268f3967b 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -521,8 +521,6 @@  long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
             ret = -ENOMEM;
             if ( dom == rover )
                 break;
-
-            rover = dom;
         }
 
         d = domain_create(dom, &op->u.createdomain, false);
@@ -534,7 +532,7 @@  long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         }
 
         ret = 0;
-        op->domain = d->domain_id;
+        rover = op->domain = d->domain_id;
         copyback = 1;
         d = NULL;
         break;