[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

On 02/06/2018 03:59 PM, Zhongze Liu wrote:
Hi Julien,


2018-02-06 21:07 GMT+08:00 Julien Grall <julien.grall@xxxxxxx>:

On 01/30/2018 05:50 PM, Zhongze Liu wrote:

Add libxl__sshm_add to map shared pages from one DomU to another, The
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.

AFAIK, a toolstack is free to not use libxlu. Someone may implement their own toolstack on top of libxl and may use ROLE_UNKNOWN by mistake.

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.

In that case, you should treat that as an error for everyone and not only xl. This would avoid confusion on other toolstack.


+/*   libxl__sshm_do_map -- map pages into slave's physmap
+ *
+ *   This functions maps
+ *     master gfn: [@msshm->begin + @sshm->offset, @msshm->end +
+ *   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

s/have to/has to/ I think.

+ *   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.

Same remark as above regarding libxlu. Note that I am maintaining the tools. Ian and Wei may have a different opinion here.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.