| Message ID | 20231110160804.29021-23-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tools: enable xenstore-stubdom to use 9pfs | expand |
Hi Juergen, On 10/11/2023 16:07, Juergen Gross wrote: > Obtain the own domid from the Xenstore special grant entry when running > as stubdom. The commit message says we would use the grant entry but ... > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > V2: > - replacement of V1 patch (ANdrew Cooper) > --- > tools/xenstored/core.c | 1 + > tools/xenstored/core.h | 1 + > tools/xenstored/minios.c | 5 +++++ > 3 files changed, 7 insertions(+) > > diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c > index 0c14823fb0..8e271e31f9 100644 > --- a/tools/xenstored/core.c > +++ b/tools/xenstored/core.c > @@ -2738,6 +2738,7 @@ static struct option options[] = { > int dom0_domid = 0; > int dom0_event = 0; > int priv_domid = 0; > +int stub_domid = -1; > > static unsigned int get_optval_uint(const char *arg) > { > diff --git a/tools/xenstored/core.h b/tools/xenstored/core.h > index d0ac587f8f..3c28007d11 100644 > --- a/tools/xenstored/core.h > +++ b/tools/xenstored/core.h > @@ -361,6 +361,7 @@ do { \ > extern int dom0_domid; > extern int dom0_event; > extern int priv_domid; > +extern int stub_domid; > extern bool keep_orphans; > > extern unsigned int timeout_watch_event_msec; > diff --git a/tools/xenstored/minios.c b/tools/xenstored/minios.c > index 0cdec3ae51..202d70387a 100644 > --- a/tools/xenstored/minios.c > +++ b/tools/xenstored/minios.c > @@ -19,6 +19,8 @@ > #include <sys/mman.h> > #include "core.h" > #include <xen/grant_table.h> > +#include <mini-os/events.h> > +#include <mini-os/gnttab.h> > > void write_pidfile(const char *pidfile) > { > @@ -56,4 +58,7 @@ void unmap_xenbus(void *interface) > > void early_init(void) > { > + stub_domid = evtchn_get_domid(); ... the function is called evtchn_*. So the commit message doesn't seem to match the code. Also, you seem to include mini-os/gnttab.h when you don't add any function that seems to be related. Lastly, shouldn't this helper be called get_domain_id() (or similar) because the domid is not specific to the event channel subsystem? Cheers,
On 10.11.23 18:36, Julien Grall wrote: > Hi Juergen, > > On 10/11/2023 16:07, Juergen Gross wrote: >> Obtain the own domid from the Xenstore special grant entry when running >> as stubdom. > The commit message says we would use the grant entry but ... Sorry, now fixed. > >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> V2: >> - replacement of V1 patch (ANdrew Cooper) >> --- >> tools/xenstored/core.c | 1 + >> tools/xenstored/core.h | 1 + >> tools/xenstored/minios.c | 5 +++++ >> 3 files changed, 7 insertions(+) >> >> diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c >> index 0c14823fb0..8e271e31f9 100644 >> --- a/tools/xenstored/core.c >> +++ b/tools/xenstored/core.c >> @@ -2738,6 +2738,7 @@ static struct option options[] = { >> int dom0_domid = 0; >> int dom0_event = 0; >> int priv_domid = 0; >> +int stub_domid = -1; >> static unsigned int get_optval_uint(const char *arg) >> { >> diff --git a/tools/xenstored/core.h b/tools/xenstored/core.h >> index d0ac587f8f..3c28007d11 100644 >> --- a/tools/xenstored/core.h >> +++ b/tools/xenstored/core.h >> @@ -361,6 +361,7 @@ do { \ >> extern int dom0_domid; >> extern int dom0_event; >> extern int priv_domid; >> +extern int stub_domid; >> extern bool keep_orphans; >> extern unsigned int timeout_watch_event_msec; >> diff --git a/tools/xenstored/minios.c b/tools/xenstored/minios.c >> index 0cdec3ae51..202d70387a 100644 >> --- a/tools/xenstored/minios.c >> +++ b/tools/xenstored/minios.c >> @@ -19,6 +19,8 @@ >> #include <sys/mman.h> >> #include "core.h" >> #include <xen/grant_table.h> >> +#include <mini-os/events.h> >> +#include <mini-os/gnttab.h> >> void write_pidfile(const char *pidfile) >> { >> @@ -56,4 +58,7 @@ void unmap_xenbus(void *interface) >> void early_init(void) >> { >> + stub_domid = evtchn_get_domid(); > > ... the function is called evtchn_*. So the commit message doesn't seem to match > the code. > > Also, you seem to include mini-os/gnttab.h when you don't add any function that > seems to be related. > > Lastly, shouldn't this helper be called get_domain_id() (or similar) because the > domid is not specific to the event channel subsystem? I've named it get_domid() now. Juergen
Hi Juergen, On 10/11/2023 16:07, Juergen Gross wrote: > Obtain the own domid from the Xenstore special grant entry when running > as stubdom. > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > V2: > - replacement of V1 patch (ANdrew Cooper) > --- > tools/xenstored/core.c | 1 + > tools/xenstored/core.h | 1 + > tools/xenstored/minios.c | 5 +++++ > 3 files changed, 7 insertions(+) > > diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c > index 0c14823fb0..8e271e31f9 100644 > --- a/tools/xenstored/core.c > +++ b/tools/xenstored/core.c > @@ -2738,6 +2738,7 @@ static struct option options[] = { > int dom0_domid = 0; > int dom0_event = 0; > int priv_domid = 0; > +int stub_domid = -1; After looking at partch #25, I feel that it would make more sense if stub_domid is a domid_t and initialized to DOMID_INVALID. At least this makes clearer that initial value is not supported to be a valid domid. Cheers,
On 13.11.23 22:59, Julien Grall wrote: > Hi Juergen, > > On 10/11/2023 16:07, Juergen Gross wrote: >> Obtain the own domid from the Xenstore special grant entry when running >> as stubdom. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> V2: >> - replacement of V1 patch (ANdrew Cooper) >> --- >> tools/xenstored/core.c | 1 + >> tools/xenstored/core.h | 1 + >> tools/xenstored/minios.c | 5 +++++ >> 3 files changed, 7 insertions(+) >> >> diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c >> index 0c14823fb0..8e271e31f9 100644 >> --- a/tools/xenstored/core.c >> +++ b/tools/xenstored/core.c >> @@ -2738,6 +2738,7 @@ static struct option options[] = { >> int dom0_domid = 0; >> int dom0_event = 0; >> int priv_domid = 0; >> +int stub_domid = -1; > > After looking at partch #25, I feel that it would make more sense if stub_domid > is a domid_t and initialized to DOMID_INVALID. > > At least this makes clearer that initial value is not supported to be a valid > domid. Yes, you are right. Juergen
diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c index 0c14823fb0..8e271e31f9 100644 --- a/tools/xenstored/core.c +++ b/tools/xenstored/core.c @@ -2738,6 +2738,7 @@ static struct option options[] = { int dom0_domid = 0; int dom0_event = 0; int priv_domid = 0; +int stub_domid = -1; static unsigned int get_optval_uint(const char *arg) { diff --git a/tools/xenstored/core.h b/tools/xenstored/core.h index d0ac587f8f..3c28007d11 100644 --- a/tools/xenstored/core.h +++ b/tools/xenstored/core.h @@ -361,6 +361,7 @@ do { \ extern int dom0_domid; extern int dom0_event; extern int priv_domid; +extern int stub_domid; extern bool keep_orphans; extern unsigned int timeout_watch_event_msec; diff --git a/tools/xenstored/minios.c b/tools/xenstored/minios.c index 0cdec3ae51..202d70387a 100644 --- a/tools/xenstored/minios.c +++ b/tools/xenstored/minios.c @@ -19,6 +19,8 @@ #include <sys/mman.h> #include "core.h" #include <xen/grant_table.h> +#include <mini-os/events.h> +#include <mini-os/gnttab.h> void write_pidfile(const char *pidfile) { @@ -56,4 +58,7 @@ void unmap_xenbus(void *interface) void early_init(void) { + stub_domid = evtchn_get_domid(); + if (stub_domid == DOMID_INVALID) + barf("could not get own domid"); }
Obtain the own domid from the Xenstore special grant entry when running as stubdom. Signed-off-by: Juergen Gross <jgross@suse.com> --- V2: - replacement of V1 patch (ANdrew Cooper) --- tools/xenstored/core.c | 1 + tools/xenstored/core.h | 1 + tools/xenstored/minios.c | 5 +++++ 3 files changed, 7 insertions(+)