Re: [Xen-devel] [PATCH v4 4/7] libxl: support mapping static shared memory areas during domain creation

[Zhongze Liu <blackskygg@xxxxxxxxx>]

Hi Julien,

2018-02-06 21:07 GMT+08:00 Julien Grall <julien.grall@xxxxxxx>:
> Hi,
>
> On 01/30/2018 05:50 PM, Zhongze Liu wrote:
>>
>> Add libxl__sshm_add to map shared pages from one DomU to another, The
>> mapping
>> process involves the follwing steps:

[...]

>> +
>> +/* Set default values for libxl_static_shm */
>> +int libxl__sshm_setdefault(libxl__gc *gc, uint32_t domid,
>> +                           libxl_static_shm *sshm)
>> +{
>> +    int rc;
>> +
>> +    if (sshm->role == LIBXL_SSHM_ROLE_UNKNOWN)
>> +        sshm->role = LIBXL_SSHM_ROLE_SLAVE;
>> +    if (sshm->prot == LIBXL_SSHM_PROT_UNKNOWN)
>> +        sshm->prot = LIBXL_SSHM_PROT_RW;
>
>
> What is the purpose of {ROLE,PROT}_UNKNOWN if you default it resp. to
> ROLE_SLAVE and PROT_RW.  Would not it be easier to just drop them?

The *_UNKNOWN values are used by the libxlu code to check whether a specific
option was set more than once. Without the default *_UNKNOWN value, I will not
be able to judge if, say, role is set to 'slave' by the user or not,
and therefore, if I
see the user setting role to 'master', I won't be able to tell if role
is specified twice
or not.

I think treating re-specification of options as errors will be good
for the users.

[...]

>> +
>> +/*   libxl__sshm_do_map -- map pages into slave's physmap
>> + *
>> + *   This functions maps
>> + *     master gfn: [@msshm->begin + @sshm->offset, @msshm->end +
>> @sshm->offset)
>> + *   into
>> + *     slave gfn: [@sshm->begin, @sshm->end)
>> + *
>> + *   The gfns of the pages that are successfully mapped will be stored
>> + *   in @mapped, and the number of the gfns will be stored in @nmapped.
>> + *
>> + *   The caller have to guarentee that sshm->begin < sshm->end and all
>> the
>
>
> s/have to/has to/ I think.
> s/guarentee/guarantee/
>
>> + *   values are page-aligned.
>
>
> Hmmm, I don't see the alignement check in libxl. So do you rely on the
> toolstack to do it?

Yes, This was done in libxlu_sshm.c.

Cheers,

Zhongze Liu

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel