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

[Juergen Gross <jgross@suse.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 1049 bytes --]

On 19.06.23 19:55, Julien Grall wrote:
> 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.

Okay, fine with me.

> 
> The rest of the changes look good to me.

Thanks,

Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]