|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 5/6] libxl: support mapping static shared memory areas during domain creation
Hi Wei,
2017-08-25 19:05 GMT+08:00 Wei Liu <wei.liu2@xxxxxxxxxx>:
> On Wed, Aug 23, 2017 at 02:08:39AM +0800, Zhongze Liu wrote:
> [...]
>> diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
>> index 5e1fc6060e..1d681d8863 100644
>> --- a/tools/libxl/libxl_arch.h
>> +++ b/tools/libxl/libxl_arch.h
>> @@ -71,6 +71,12 @@ int libxl__arch_extra_memory(libxl__gc *gc,
>> const libxl_domain_build_info *info,
>> uint64_t *out);
>>
>> +_hidden
>> +bool libxl__arch_domain_support_sshm(const libxl_domain_build_info *b_info);
>
> No need to take any argument afaict.
This is needed because later on, we will restrict this to HVM only on
x86, we need b_info
to tell if a domain is HVM or not.
>
> [...]
>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>> index 1158303e1a..8e5ec486d2 100644
>> --- a/tools/libxl/libxl_create.c
>> +++ b/tools/libxl/libxl_create.c
>> @@ -501,6 +501,14 @@ int libxl__domain_build(libxl__gc *gc,
>> ret = ERROR_INVAL;
>> goto out;
>> }
>> +
>> + /* the p2m has been setup, we could map the static shared memory now. */
>> + ret = libxl__sshm_add(gc, domid, d_config->sshms, d_config->num_sshms);
>> + if (ret != 0) {
>> + LOG(ERROR, "failed to map static shared memory");
>> + goto out;
>> + }
>> +
>> ret = libxl__build_post(gc, domid, info, state, vments, localents);
>> out:
>> return ret;
>> @@ -918,6 +926,25 @@ static void initiate_domain_create(libxl__egc *egc,
>> goto error_out;
>> }
>>
>> + if (d_config->num_sshms != 0 &&
>> + !libxl__arch_domain_support_sshm(&d_config->b_info)) {
>> + LOGD(ERROR, domid, "static_shm is not supported by this domain
>> type.");
>> + ret = ERROR_INVAL;
>> + goto error_out;
>> + }
>> +
>> + ret = libxl__sshm_check_overlap(gc, domid,
>> + d_config->sshms, d_config->num_sshms);
>> + if (ret) goto error_out;
>
> Better move this after setting default.
OK.
>
>> +
>> + for (i = 0; i < d_config->num_sshms; ++i) {
>> + ret = libxl__sshm_setdefault(gc, domid, &d_config->sshms[i]);
>> + if (ret) {
>> + LOGD(ERROR, domid, "Unable to set defaults for static sshm");
>> + goto error_out;
>> + }
>> + }
>> +
>> ret = libxl__domain_make(gc, d_config, &domid, &state->config);
>> if (ret) {
>> LOGD(ERROR, domid, "cannot make domain: %d", ret);
>> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
>> index 724750967c..74bc0acb21 100644
> [...]
>> diff --git a/tools/libxl/libxl_sshm.c b/tools/libxl/libxl_sshm.c
>> new file mode 100644
>> index 0000000000..e16c24ccb9
>> --- /dev/null
>> +++ b/tools/libxl/libxl_sshm.c
>> @@ -0,0 +1,336 @@
>> +#include "libxl_osdeps.h"
>> +#include "libxl_internal.h"
>> +#include "libxl_arch.h"
>> +
>> +#define SSHM_ERROR(domid, sshmid, f, ...) \
>> + LOGD(ERROR, domid, "static_shm id = %s: " f, sshmid, ##__VA_ARGS__)
>> +
>> +
>> +/* Set default values for libxl_static_shm */
>> +int libxl__sshm_setdefault(libxl__gc *gc, uint32_t domid,
>> + libxl_static_shm *sshm)
>
> Indentation.
>
>> +{
>> + 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;
>> + }
>
> You can avoid using {} when there is only one statement.
>
>> + /* role-specific checks */
>> + if (LIBXL_SSHM_ROLE_SLAVE == sshm->role) {
>
> Style.
Sorry about this. I've realized this and have removed all the Yoda
conditions in my new draft.
>
>> + if (sshm->offset == LIBXL_SSHM_RANGE_UNKNOWN) {
>> + sshm->offset = 0;
>> + }
>> + if (sshm->cache_policy != LIBXL_SSHM_CACHEPOLICY_UNKNOWN) {
>> + SSHM_ERROR(domid, sshm->id,
>> + "cache_policy is only applicable to master domains");
>> + return ERROR_INVAL;
>> + }
>> + } else {
>> + if (sshm->offset != LIBXL_SSHM_RANGE_UNKNOWN) {
>> + SSHM_ERROR(domid, sshm->id,
>> + "offset is only applicable to slave domains");
>> + return ERROR_INVAL;
>> + }
>> + rc = libxl__arch_domain_sshm_cachepolicy_setdefault(sshm);
>> + if (rc) {
>> + SSHM_ERROR(domid, sshm->id,
>> + "cache policy not supported on this platform");
>> + return ERROR_INVAL;
>> + }
>> + }
>
> Please use goto style error handling.
>
>> +
>> + return 0;
>> +}
>> +
>> +/* Compare function for sorting sshm ranges by sshm->begin */
>> +static int sshm_range_cmp(const void *a, const void *b)
>> +{
>> + libxl_static_shm *const *sshma = a, *const *sshmb = b;
>> + return (*sshma)->begin > (*sshmb)->begin ? 1 : -1;
>> +}
>> +
>> +/* check if the sshm slave configs in @sshm overlap */
>> +int libxl__sshm_check_overlap(libxl__gc *gc, uint32_t domid,
>> + libxl_static_shm *sshms, int len)
>> +{
>> +
>> + const libxl_static_shm **slave_sshms = NULL;
>> + int num_slaves;
>> + int i;
>> +
>> + slave_sshms = libxl__calloc(gc, len, sizeof(slave_sshms[0]));
>> + num_slaves = 0;
>> + for (i = 0; i < len; ++i) {
>> + if (LIBXL_SSHM_ROLE_SLAVE == sshms[i].role)
>
> Style.
>
>> +/* The caller have to guarentee that sshm->begin < sshm->end */
>> +static int libxl__sshm_do_map(libxl__gc *gc, uint32_t mid, uint32_t sid,
>> + libxl_static_shm *sshm,
>> + uint64_t mbegin, uint64_t mend)
>> +{
>> + int rc;
>> + int i;
>> + unsigned int num_mpages, num_spages, offset;
>> + int *errs;
>> + xen_ulong_t *idxs;
>> + xen_pfn_t *gpfns;
>> +
>> + num_mpages = (mend - mbegin) >> 12;
>> + num_spages = (sshm->end - sshm->begin) >> 12;
>> + offset = sshm->offset >> 12;
>> +
>> + /* Check range. Test offset < mpages first to avoid overflow */
>> + if ((offset >= num_mpages) || (num_mpages - offset < num_spages)) {
>> + SSHM_ERROR(sid, sshm->id, "exceeds master's address space.");
>> + rc = ERROR_INVAL;
>> + goto out;
>> + }
>> +
>> + /* fill out the pfn's and do the mapping */
>> + errs = libxl__calloc(gc, num_spages, sizeof(int));
>> + idxs = libxl__calloc(gc, num_spages, sizeof(xen_ulong_t));
>> + gpfns = libxl__calloc(gc, num_spages, sizeof(xen_pfn_t));
>> + for (i = 0; i < num_spages; i++) {
>> + idxs[i] = (mbegin >> 12) + offset + i;
>> + gpfns[i]= (sshm->begin >> 12) + i;
>> + }
>> + rc = xc_domain_add_to_physmap_batch(CTX->xch,
>> + sid, mid,
>> + XENMAPSPACE_gmfn_foreign,
>> + num_spages,
>> + idxs, gpfns, errs);
>> +
>> + for (i = 0; i< num_spages; i++) {
>> + if (errs[i]) {
>> + SSHM_ERROR(sid, sshm->id,
>> + "can't map at address 0x%"PRIx64".",
>> + sshm->begin + (offset << 12) );
>> + rc = ERROR_FAIL;
>> + }
>> + }
>> + if (rc) goto out;
>> +
>
> How is partial failure handled?
Em...
If one of the pages failed, the whole domain won't be constructed. But the
mapped pages are still occupying the refcount.
Do you think I should continue or just throw out a warning and let to user to
decide whether she is going to destroy it or not?
>
>> + rc = 0;
>> +
>> +out:
>> + return rc;
>> +}
>> +
>> +static int libxl__sshm_add_slave(libxl__gc *gc, uint32_t domid,
>> + libxl_static_shm *sshm)
>> +{
> [...]
>> +
>> + /* check if the slave is asking too much permission */
>> + if (LIBXL_SSHM_PROT_UNKNOWN == sshm->prot) {
>> + sshm->prot = master_sshm.prot;
>> + }
>
> Style.
>
>> +int libxl__sshm_add(libxl__gc *gc, uint32_t domid,
>> + libxl_static_shm *sshms, int len)
>> +{
>> + int rc, i;
>> +
>> + if (!len) return 0;
>
>
will remove this.
>
>> +
>> + for (i = 0; i < len; ++i) {
>> + if (LIBXL_SSHM_ROLE_SLAVE == sshms[i].role) {
>> + rc = libxl__sshm_add_slave(gc, domid, sshms+i);
>> + } else {
>> + rc = libxl__sshm_add_master(gc, domid, sshms+i);
>> + }
>> + if (rc) return rc;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-basic-offset: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
> [...]
>> diff --git a/tools/libxl/libxl_xshelp.c b/tools/libxl/libxl_xshelp.c
>> index c4a18df353..d91fbf5fda 100644
>> --- a/tools/libxl/libxl_xshelp.c
>> +++ b/tools/libxl/libxl_xshelp.c
>> @@ -193,6 +193,14 @@ char *libxl__xs_libxl_path(libxl__gc *gc, uint32_t
>> domid)
>> return s;
>> }
>>
>> +char *libxl__xs_get_sshmpath(libxl__gc *gc, const char *id)
>> +{
>> + char *s = GCSPRINTF("/local/static_shm/%s", id);
>
>
> This can't fail.
>
>> + if (!s)
>> + LOGE(ERROR, "cannot allocate static shm path");
>> + return s;
>> +}
>> +
>> int libxl__xs_read_mandatory(libxl__gc *gc, xs_transaction_t t,
>> const char *path, const char **result_out)
>> {
>> --
>> 2.14.0
>>
Cheers,
Zhongze Liu.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |