|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] libxl/xl: add support for Xen 9pfs
On Mon, 27 Mar 2017, Wei Liu wrote:
> On Thu, Mar 23, 2017 at 04:36:19PM -0700, Stefano Stabellini wrote:
> > docs/man/xl.cfg.pod.5.in | 31 +++++++++++++
> > tools/libxl/Makefile | 2 +-
> > tools/libxl/libxl.h | 10 +++++
> > tools/libxl/libxl_9pfs.c | 87
> > ++++++++++++++++++++++++++++++++++++
> > tools/libxl/libxl_create.c | 3 ++
> > tools/libxl/libxl_internal.h | 6 +++
> > tools/libxl/libxl_types.idl | 10 +++++
> > tools/libxl/libxl_types_internal.idl | 1 +
> > tools/xl/xl_parse.c | 55 ++++++++++++++++++++++-
> > 9 files changed, 203 insertions(+), 2 deletions(-)
> > create mode 100644 tools/libxl/libxl_9pfs.c
> >
> > diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
> > index 505c111..699a644 100644
> > --- a/docs/man/xl.cfg.pod.5.in
> > +++ b/docs/man/xl.cfg.pod.5.in
> > @@ -516,6 +516,37 @@ value is optional if this is a guest domain.
> >
> > =back
> >
> > +=item B<xen_9pfs=[ "XEN_9PFS_SPEC_STRING", "XEN_9PFS_SPEC_STRING", ...]>
> > +
>
> Let me guess, the reason for xen_ prefix is 9pfs isn't really a valid
> identifier in C.
>
> Can we get rid of the xen_ prefix by naming everything p9* ? I think
> that's how QEMU does it.
OK. I'll rename the VM config option to 9pfs, and all the internal
functions and variables to 9p.
> > +Creates a Xen 9pfs connection to share a filesystem from backend to
> > +frontend.
> > +
> > +Each B<XEN_9PFS_SPEC_STRING> is a comma-separated list of C<KEY=VALUE>
> > +settings, from the following list:
> > +
> > +=over 4
> > +
> > +=item C<tag=STRING>
> > +
> > +9pfs tag to identify the filesystem share. The tag is needed on the
> > +guest side to mount it.
> > +
> > +=item C<security_model="none">
> > +
> > +Only "none" is supported today, which means that files are stored using
> > +the same credentials as they are created on the guest (no user ownership
> > +squash or remap).
> > +
>
> What is the difficulty for supporting other modes as well?
>
> I think it is just passing through the string to QEMU, right?
It is also testing and validation. Also, the current version of the
protocol only supports "none".
> > +=item C<path=STRING>
> > +
> > +Filesystem path on the backend to export.
> > +
> > +=item C<backend=DOMAIN>
> > +
> > +Specify the backend domain name or id, defaults to dom0.
> > +
> > +=back
> > +
> > =item B<vfb=[ "VFB_SPEC_STRING", "VFB_SPEC_STRING", ...]>
> >
> [...]
> > diff --git a/tools/libxl/libxl_9pfs.c b/tools/libxl/libxl_9pfs.c
> > new file mode 100644
> > index 0000000..8cb0772
> > --- /dev/null
> > +++ b/tools/libxl/libxl_9pfs.c
> > @@ -0,0 +1,87 @@
> > +/*
> > + * Copyright (C) 2017 Aporeto
> > + * Author Stefano Stabellini <stefano@xxxxxxxxxxx>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU Lesser General Public License as published
> > + * by the Free Software Foundation; version 2.1 only. with the special
> > + * exception on linking described in file LICENSE.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU Lesser General Public License for more details.
> > + */
> > +
> > +#include "libxl_osdeps.h"
> > +
> > +#include "libxl_internal.h"
> > +
> > +int libxl__device_xen_9pfs_setdefault(libxl__gc *gc, libxl_device_xen_9pfs
> > *xen_9pfs)
> > +{
> > + int rc;
> > +
> > + rc = libxl__resolve_domid(gc, xen_9pfs->backend_domname,
> > &xen_9pfs->backend_domid);
>
> These lines are too long.
I'll fix
> > + return rc;
> > +}
> > +
> > +static int libxl__device_from_xen_9pfs(libxl__gc *gc, uint32_t domid,
> > + libxl_device_xen_9pfs *xen_9pfs,
> > + libxl__device *device)
> > +{
> > + device->backend_devid = xen_9pfs->devid;
> > + device->backend_domid = xen_9pfs->backend_domid;
> > + device->backend_kind = LIBXL__DEVICE_KIND_9PFS;
> > + device->devid = xen_9pfs->devid;
> > + device->domid = domid;
> > + device->kind = LIBXL__DEVICE_KIND_9PFS;
> > +
> > + return 0;
> > +}
> > +
> > +
> > +int libxl__device_xen_9pfs_add(libxl__gc *gc, uint32_t domid,
> > + libxl_device_xen_9pfs *xen_9pfs)
>
> Indentation.
I'll fix
> > +{
> > + flexarray_t *front;
> > + flexarray_t *back;
> > + libxl__device device;
> > + int rc;
> > +
> > + rc = libxl__device_xen_9pfs_setdefault(gc, xen_9pfs);
> > + if (rc) goto out;
> > +
> > + front = flexarray_make(gc, 16, 1);
> > + back = flexarray_make(gc, 16, 1);
> > +
> > + if (xen_9pfs->devid == -1) {
> > + if ((xen_9pfs->devid = libxl__device_nextid(gc, domid, "9pfs")) <
> > 0) {
> > + rc = ERROR_FAIL;
> > + goto out;
> > + }
> > + }
> > +
> > + rc = libxl__device_from_xen_9pfs(gc, domid, xen_9pfs, &device);
> > + if (rc != 0) goto out;
> > +
> > + flexarray_append_pair(back, "frontend-id", libxl__sprintf(gc, "%d",
> > domid));
> > + flexarray_append_pair(back, "online", "1");
> > + flexarray_append_pair(back, "state", GCSPRINTF("%d",
> > XenbusStateInitialising));
> > + flexarray_append_pair(front, "backend-id",
> > + libxl__sprintf(gc, "%d",
> > xen_9pfs->backend_domid));
> > + flexarray_append_pair(front, "state", GCSPRINTF("%d",
> > XenbusStateInitialising));
> > + flexarray_append_pair(front, "tag", xen_9pfs->tag);
> > + flexarray_append_pair(back, "path", xen_9pfs->path);
> > + flexarray_append_pair(back, "security_model",
> > xen_9pfs->security_model);
> > +
> > + libxl__device_generic_add(gc, XBT_NULL, &device,
> > + libxl__xs_kvs_of_flexarray(gc, back),
> > + libxl__xs_kvs_of_flexarray(gc, front),
> > + NULL);
> > + rc = 0;
> > +out:
> > + return rc;
> > +}
> > +
> > +LIBXL_DEFINE_DEVICE_REMOVE(xen_9pfs)
> > +
> [...]
> > libxl__console_backend = Enumeration("console_backend", [
> > diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> > index 1ef0c27..275b72f 100644
> > --- a/tools/xl/xl_parse.c
> > +++ b/tools/xl/xl_parse.c
> > @@ -716,7 +716,7 @@ void parse_config_data(const char *config_source,
> > long l, vcpus = 0;
> > XLU_Config *config;
> > XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *vtpms,
> > - *usbctrls, *usbdevs;
> > + *usbctrls, *usbdevs, *xen_9pfs_opts;
>
> Maybe make the name more like the others?
I'll do
> Other than these superficial issues, the code looks correct to me.
OK, thanks
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |