On 10.11.23 18:31, Julien Grall wrote:
> Hi Juergen,
>
> On 10/11/2023 16:07, Juergen Gross wrote:
>> Some xenstored initialization needs to be done in the daemon case only,
>> so split it out into a new early_init() function being a stub in the
>> stubdom case.
>
> It is not entirely clear to me how you decided the split. For example...
>
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
>> ---
>> V2:
>> - rename function
>> - move patch earlier in the series
>> ---
>> tools/xenstored/core.c | 6 +-----
>> tools/xenstored/core.h | 3 +++
>> tools/xenstored/minios.c | 3 +++
>> tools/xenstored/posix.c | 11 +++++++++++
>> 4 files changed, 18 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
>> index edd07711db..0c14823fb0 100644
>> --- a/tools/xenstored/core.c
>> +++ b/tools/xenstored/core.c
>> @@ -2933,11 +2933,7 @@ int main(int argc, char *argv[])
>> if (optind != argc)
>> barf("%s: No arguments desired", argv[0]);
>> - reopen_log();
>> -
>> - /* Make sure xenstored directory exists. */
>> - /* Errors ignored here, will be reported when we open files */
>> - mkdir(xenstore_daemon_rundir(), 0755);
>> + early_init();
>> if (dofork) {
>> openlog("xenstored", 0, LOG_DAEMON);
>
> For stubdom we would not fork, so I would expect the call to openlog() not
> necessary. Same for the init_pipe() below.
The main goal was to move the "mkdir(xenstore_daemon_rundir(), 0755);" out of
the way for stubdom.
I'll make this more clear in the commit message, ...
>
>> diff --git a/tools/xenstored/core.h b/tools/xenstored/core.h
>> index 480b0f5f7b..d0ac587f8f 100644
>> --- a/tools/xenstored/core.h
>> +++ b/tools/xenstored/core.h
>> @@ -35,6 +35,8 @@
>> #include "list.h"
>> #include "hashtable.h"
>> +#define XENSTORE_LIB_DIR XEN_LIB_DIR "/xenstore"
>> +
>> #ifndef O_CLOEXEC
>> #define O_CLOEXEC 0
>> /* O_CLOEXEC support is needed for Live Update in the daemon case. */
>> @@ -384,6 +386,7 @@ static inline bool domain_is_unprivileged(const struct
>> connection *conn)
>> /* Return the event channel used by xenbus. */
>> evtchn_port_t get_xenbus_evtchn(void);
>> +void early_init(void);
>> /* Write out the pidfile */
>> void write_pidfile(const char *pidfile);
>> diff --git a/tools/xenstored/minios.c b/tools/xenstored/minios.c
>> index 0779efbf91..0cdec3ae51 100644
>> --- a/tools/xenstored/minios.c
>> +++ b/tools/xenstored/minios.c
>> @@ -54,3 +54,6 @@ void unmap_xenbus(void *interface)
>> xengnttab_unmap(*xgt_handle, interface, 1);
>> }
>> +void early_init(void)
>> +{
>> +}
>> diff --git a/tools/xenstored/posix.c b/tools/xenstored/posix.c
>> index 7e03dd982d..fcb7791bd7 100644
>> --- a/tools/xenstored/posix.c
>> +++ b/tools/xenstored/posix.c
>> @@ -22,6 +22,7 @@
>> #include <fcntl.h>
>> #include <stdlib.h>
>> #include <sys/mman.h>
>> +#include <xen-tools/xenstore-common.h>
>> #include "utils.h"
>> #include "core.h"
>> @@ -157,3 +158,13 @@ void *xenbus_map(void)
>> return addr;
>> }
>> +
>> +void early_init(void)
>> +{
>> + reopen_log();
>> +
>> + /* Make sure xenstored directories exist. */
>> + /* Errors ignored here, will be reported when we open files */
>> + mkdir(xenstore_daemon_rundir(), 0755);
>> + mkdir(XENSTORE_LIB_DIR, 0755);
>
> The addition of the second mkdir() doesn't seem to be explained in the commit
> message.
... moving this change into context.
I'll look into moving more non-stubdom function calls into early_init() in
posix.c.
Juergen