Re: [PATCH v3 14/16] tools/xenstore: split out environment specific live update code

[Julien Grall <julien@xxxxxxx>]

Hi Juergen,

On 30/05/2023 09:54, Juergen Gross wrote:
> -static struct live_update *lu_status;
> -
> -struct lu_dump_state {
> -       void *buf;
> -       unsigned int size;
> -#ifndef __MINIOS__
> -       int fd;
> -       char *filename;
> -#endif
> -};
> -
> -static int lu_destroy(void *data)
> -{
> -#ifdef __MINIOS__
> -       if (lu_status->dump_state)
> -               munmap(lu_status->dump_state, lu_status->dump_size);
> -#endif
> -       lu_status = NULL;
> -
> -       return 0;
> -}
I think moving all lu_destroy() out of xenstored_control.c is a mistake 
because we now need to remember that any common change in lu_begin() may 
need an update in the two implementation of lu_destroy().
Even if this seems pointless for a few lines, it would be best to split 
lu_destroy() in two parts: one common and one per lu backend.
The rest of the changes look good to me.

Cheers,

--
Julien Grall