Re: [PATCH v2 5/6] libxl: support mapping static shared memory areas during domain creation

[Zhongze Liu <blackskygg@gmail.com>]

Hi Wei,

Thanks for reviewing.

2017-09-02 0:23 GMT+08:00 Wei Liu <wei.liu2@citrix.com>:
> On Sun, Aug 27, 2017 at 04:36:14PM +0800, Zhongze Liu wrote:
>> Add libxl__sshm_add to map shared pages from one DomU to another, The mapping
>> process involves the follwing steps:
>>
>>   * Set defaults and check for further errors in the static_shm configs:
>>     overlapping areas, invalid ranges, duplicated master domain,
>>     no master domain etc.
>>   * Write infomation of static shared memory areas into the appropriate
>>     xenstore paths.
>>   * Use xc_domain_add_to_physmap_batch to do the page sharing.
>>   * Set the refcount of the shared region accordingly
>>
>> Temporarily mark this as unsupported on x86 because calling p2m_add_foregin on
>> two domU's is currently not allowd on x86 (see the comments in
>> x86/mm/p2m.c:p2m_add_foregin for more details).
>>
>> This is for the proposal "Allow setting up shared memory areas between VMs
>> from xl config file" (see [1]).
>>
>> [1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html
>>
>> Signed-off-by: Zhongze Liu <blackskygg@gmail.com>
> [...]
>> +
>> +        rc = libxl__xs_read_checked(gc, xt,
>> +                                    GCSPRINTF("%s/master", sshm_path),
>> +                                    &xs_value);
>> +        if (rc) goto out;
>> +        master_domid = strtoull(xs_value, NULL, 16);
>> +
>> +        if (sshm->prot == LIBXL_SSHM_PROT_UNKNOWN) {
>> +            sshm->prot = master_sshm.prot;
>> +        }
>> +        /* check if the slave is asking too much permission */
>> +        if (master_sshm.prot < sshm->prot) {
>> +            SSHM_ERROR(domid, sshm->id, "slave is asking too much permission.");
>> +            rc = ERROR_INVAL;
>> +            goto out;
>> +        }
>> +
>> +        /* all checks passed, do the job */
>> +        if (!isretry) {
>> +            rc = libxl__sshm_do_map(gc, master_domid, domid, sshm,
>> +                                    master_sshm.begin, master_sshm.end);
>> +            if (rc) goto out;
>
> You probably need to roll back the mapping should the transaction gets
> aborted in this loop.

OK. I'll move the rollback code to the abort path.

>
>> +        }
>> +
>> +        /* write the result to xenstore and commit */
>> +        rc = libxl__xs_write_checked(gc, xt, dom_role_path, "slave");
>> +        if (rc) goto out;
>> +        rc = libxl__xs_writev(gc, xt, slave_path, ents);
>> +        if (rc) goto out;
>> +        rc = libxl__sshm_incref(gc, xt, sshm_path);
>> +        if (rc) goto out;
>> +
>> +        rc = libxl__xs_transaction_commit(gc, &xt);
>> +        if (!rc) break;
>> +        if (rc < 0) goto out;
>> +        isretry = true;
>> +    }
>> +
>> +    rc = 0;
>> +out:
>> +    libxl__xs_transaction_abort(gc, &xt);
>> +    return rc;
>> +}
>> +
>> +static int libxl__sshm_add_master(libxl__gc *gc, uint32_t domid,
>> +                                  libxl_static_shm *sshm)
>> +{
>> +
>
> No refcount increment here?

For every sshm ID, this will be called only once. And in the xenstore
ents array, I've  already
set "users = 1", so there's no need to call libxl__sshm_incref().


Cheers,

Zhongze Liu

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel