| Message ID | 1459347035-32740-1-git-send-email-jgross@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 30 Mar 2016, Juergen Gross wrote: > Add a Xenstore directory for each supported pv backend. This will allow > Xen tools to decide which backend type to use in case there are > multiple possibilities. > > The information is added under > /local/domain/<backend-domid>/device-model/<domid>/backends > before the "running" state is written to Xenstore. Using a directory > for each directory enables us to add parameters for specific backends > in the future. > > In order to reuse the Xenstore directory creation already present in > hw/xen/xen_devconfig.c move the related functions to > hw/xen/xen_backend.c where they fit better. An interface change like this one, needs a new file under docs (in the Xen repository) to document it. Please add a reference to it, in the commit message here. In addition please make sure that this can work with QEMU running as non-root. What are the permissions of the new xenstore directory? > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > hw/xen/xen_backend.c | 52 ++++++++++++++++++++++++++++++++++++++++++++ > hw/xen/xen_devconfig.c | 52 ++------------------------------------------ > include/hw/xen/xen_backend.h | 1 + > 3 files changed, 55 insertions(+), 50 deletions(-) > > diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c > index 60575ad..8983440 100644 > --- a/hw/xen/xen_backend.c > +++ b/hw/xen/xen_backend.c > @@ -42,11 +42,35 @@ struct xs_handle *xenstore = NULL; > const char *xen_protocol; > > /* private */ > +struct xs_dirs { > + char *xs_dir; > + QTAILQ_ENTRY(xs_dirs) list; > +}; > +static QTAILQ_HEAD(xs_dirs_head, xs_dirs) xs_cleanup = QTAILQ_HEAD_INITIALIZER(xs_cleanup); > + > static QTAILQ_HEAD(XenDeviceHead, XenDevice) xendevs = QTAILQ_HEAD_INITIALIZER(xendevs); > static int debug = 0; > > /* ------------------------------------------------------------- */ > > +static void xenstore_cleanup_dir(char *dir) > +{ > + struct xs_dirs *d; > + > + d = g_malloc(sizeof(*d)); > + d->xs_dir = dir; > + QTAILQ_INSERT_TAIL(&xs_cleanup, d, list); > +} > + > +void xen_config_cleanup(void) > +{ > + struct xs_dirs *d; > + > + QTAILQ_FOREACH(d, &xs_cleanup, list) { > + xs_rm(xenstore, 0, d->xs_dir); > + } > +} > + > int xenstore_write_str(const char *base, const char *node, const char *val) > { > char abspath[XEN_BUFSIZE]; > @@ -75,6 +99,28 @@ char *xenstore_read_str(const char *base, const char *node) > return ret; > } > > +int xenstore_mkdir(char *path, int p) > +{ > + struct xs_permissions perms[2] = {{ > + .id = 0, /* set owner: dom0 */ > + },{ > + .id = xen_domid, > + .perms = p, > + }}; > + > + if (!xs_mkdir(xenstore, 0, path)) { > + xen_be_printf(NULL, 0, "xs_mkdir %s: failed\n", path); > + return -1; > + } > + xenstore_cleanup_dir(g_strdup(path)); > + > + if (!xs_set_permissions(xenstore, 0, path, perms, 2)) { > + xen_be_printf(NULL, 0, "xs_set_permissions %s: failed\n", path); > + return -1; > + } > + return 0; > +} > + > int xenstore_write_int(const char *base, const char *node, int ival) > { > char val[12]; > @@ -726,6 +772,12 @@ err: > > int xen_be_register(const char *type, struct XenDevOps *ops) > { > + char path[50]; > + > + snprintf(path, sizeof (path), "device-model/%u/backends/%s", xen_domid, > + type); > + xenstore_mkdir(path, XS_PERM_READ); > + > return xenstore_scan(type, xen_domid, ops); > } > > diff --git a/hw/xen/xen_devconfig.c b/hw/xen/xen_devconfig.c > index 1f30fe4..b7d290d 100644 > --- a/hw/xen/xen_devconfig.c > +++ b/hw/xen/xen_devconfig.c > @@ -5,54 +5,6 @@ > > /* ------------------------------------------------------------- */ > > -struct xs_dirs { > - char *xs_dir; > - QTAILQ_ENTRY(xs_dirs) list; > -}; > -static QTAILQ_HEAD(xs_dirs_head, xs_dirs) xs_cleanup = QTAILQ_HEAD_INITIALIZER(xs_cleanup); > - > -static void xen_config_cleanup_dir(char *dir) > -{ > - struct xs_dirs *d; > - > - d = g_malloc(sizeof(*d)); > - d->xs_dir = dir; > - QTAILQ_INSERT_TAIL(&xs_cleanup, d, list); > -} > - > -void xen_config_cleanup(void) > -{ > - struct xs_dirs *d; > - > - QTAILQ_FOREACH(d, &xs_cleanup, list) { > - xs_rm(xenstore, 0, d->xs_dir); > - } > -} > - > -/* ------------------------------------------------------------- */ > - > -static int xen_config_dev_mkdir(char *dev, int p) > -{ > - struct xs_permissions perms[2] = {{ > - .id = 0, /* set owner: dom0 */ > - },{ > - .id = xen_domid, > - .perms = p, > - }}; > - > - if (!xs_mkdir(xenstore, 0, dev)) { > - xen_be_printf(NULL, 0, "xs_mkdir %s: failed\n", dev); > - return -1; > - } > - xen_config_cleanup_dir(g_strdup(dev)); > - > - if (!xs_set_permissions(xenstore, 0, dev, perms, 2)) { > - xen_be_printf(NULL, 0, "xs_set_permissions %s: failed\n", dev); > - return -1; > - } > - return 0; > -} > - > static int xen_config_dev_dirs(const char *ftype, const char *btype, int vdev, > char *fe, char *be, int len) > { > @@ -66,8 +18,8 @@ static int xen_config_dev_dirs(const char *ftype, const char *btype, int vdev, > snprintf(be, len, "%s/backend/%s/%d/%d", dom, btype, xen_domid, vdev); > free(dom); > > - xen_config_dev_mkdir(fe, XS_PERM_READ | XS_PERM_WRITE); > - xen_config_dev_mkdir(be, XS_PERM_READ); > + xenstore_mkdir(fe, XS_PERM_READ | XS_PERM_WRITE); > + xenstore_mkdir(be, XS_PERM_READ); > return 0; > } > > diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h > index b4b4ff0..1bc5042 100644 > --- a/include/hw/xen/xen_backend.h > +++ b/include/hw/xen/xen_backend.h > @@ -63,6 +63,7 @@ extern const char *xen_protocol; > extern DeviceState *xen_sysdev; > > /* xenstore helper functions */ > +int xenstore_mkdir(char *path, int p); > int xenstore_write_str(const char *base, const char *node, const char *val); > int xenstore_write_int(const char *base, const char *node, int ival); > int xenstore_write_int64(const char *base, const char *node, int64_t ival); > -- > 2.6.2 >
On 01/04/16 16:56, Stefano Stabellini wrote: > On Wed, 30 Mar 2016, Juergen Gross wrote: >> Add a Xenstore directory for each supported pv backend. This will allow >> Xen tools to decide which backend type to use in case there are >> multiple possibilities. >> >> The information is added under >> /local/domain/<backend-domid>/device-model/<domid>/backends >> before the "running" state is written to Xenstore. Using a directory >> for each directory enables us to add parameters for specific backends >> in the future. >> >> In order to reuse the Xenstore directory creation already present in >> hw/xen/xen_devconfig.c move the related functions to >> hw/xen/xen_backend.c where they fit better. > > An interface change like this one, needs a new file under docs (in the > Xen repository) to document it. Please add a reference to it, in the > commit message here. Is the Xenstore path documentation enough? Or do you want something like docs/misc/qemu-deprivilege.txt to be added? > In addition please make sure that this can work with QEMU running as > non-root. What are the permissions of the new xenstore directory? Should work, as I'm using the same functions as any backend in qemu is using. The new directory is read-only for the qemu target domain (same as the backend directories). Juergen
On 30/03/16 15:10, Juergen Gross wrote: > Add a Xenstore directory for each supported pv backend. This will allow > Xen tools to decide which backend type to use in case there are > multiple possibilities. > > The information is added under > /local/domain/<backend-domid>/device-model/<domid>/backends > before the "running" state is written to Xenstore. Using a directory > for each directory enables us to add parameters for specific backends > in the future. "Using a directory for each backend..."? > > In order to reuse the Xenstore directory creation already present in > hw/xen/xen_devconfig.c move the related functions to > hw/xen/xen_backend.c where they fit better. Why Xenstore and not QMP? David
On 04/04/16 11:18, David Vrabel wrote: > On 30/03/16 15:10, Juergen Gross wrote: >> Add a Xenstore directory for each supported pv backend. This will allow >> Xen tools to decide which backend type to use in case there are >> multiple possibilities. >> >> The information is added under >> /local/domain/<backend-domid>/device-model/<domid>/backends >> before the "running" state is written to Xenstore. Using a directory >> for each directory enables us to add parameters for specific backends >> in the future. > > "Using a directory for each backend..."? Yes. As I've written: this enables us to enhance the interface later without the need to modify it first. >> In order to reuse the Xenstore directory creation already present in >> hw/xen/xen_devconfig.c move the related functions to >> hw/xen/xen_backend.c where they fit better. > > Why Xenstore and not QMP? We need to communicate with qemu only once instead of each time a device is added to the domain. Doing the communication via Xenstore is much simpler. Juergen
On Mon, 4 Apr 2016, Juergen Gross wrote: > On 01/04/16 16:56, Stefano Stabellini wrote: > > On Wed, 30 Mar 2016, Juergen Gross wrote: > >> Add a Xenstore directory for each supported pv backend. This will allow > >> Xen tools to decide which backend type to use in case there are > >> multiple possibilities. > >> > >> The information is added under > >> /local/domain/<backend-domid>/device-model/<domid>/backends > >> before the "running" state is written to Xenstore. Using a directory > >> for each directory enables us to add parameters for specific backends > >> in the future. > >> > >> In order to reuse the Xenstore directory creation already present in > >> hw/xen/xen_devconfig.c move the related functions to > >> hw/xen/xen_backend.c where they fit better. > > > > An interface change like this one, needs a new file under docs (in the > > Xen repository) to document it. Please add a reference to it, in the > > commit message here. > > Is the Xenstore path documentation enough? Or do you want something > like docs/misc/qemu-deprivilege.txt to be added? Something like qemu-deprivilege.txt would be acceptable. > > In addition please make sure that this can work with QEMU running as > > non-root. What are the permissions of the new xenstore directory? > > Should work, as I'm using the same functions as any backend in qemu is > using. The new directory is read-only for the qemu target domain (same > as the backend directories). All right, just make sure to have xen-qemuuser-shared or xen-qemuuser-domid on your system when you test :-)
diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c index 60575ad..8983440 100644 --- a/hw/xen/xen_backend.c +++ b/hw/xen/xen_backend.c @@ -42,11 +42,35 @@ struct xs_handle *xenstore = NULL; const char *xen_protocol; /* private */ +struct xs_dirs { + char *xs_dir; + QTAILQ_ENTRY(xs_dirs) list; +}; +static QTAILQ_HEAD(xs_dirs_head, xs_dirs) xs_cleanup = QTAILQ_HEAD_INITIALIZER(xs_cleanup); + static QTAILQ_HEAD(XenDeviceHead, XenDevice) xendevs = QTAILQ_HEAD_INITIALIZER(xendevs); static int debug = 0; /* ------------------------------------------------------------- */ +static void xenstore_cleanup_dir(char *dir) +{ + struct xs_dirs *d; + + d = g_malloc(sizeof(*d)); + d->xs_dir = dir; + QTAILQ_INSERT_TAIL(&xs_cleanup, d, list); +} + +void xen_config_cleanup(void) +{ + struct xs_dirs *d; + + QTAILQ_FOREACH(d, &xs_cleanup, list) { + xs_rm(xenstore, 0, d->xs_dir); + } +} + int xenstore_write_str(const char *base, const char *node, const char *val) { char abspath[XEN_BUFSIZE]; @@ -75,6 +99,28 @@ char *xenstore_read_str(const char *base, const char *node) return ret; } +int xenstore_mkdir(char *path, int p) +{ + struct xs_permissions perms[2] = {{ + .id = 0, /* set owner: dom0 */ + },{ + .id = xen_domid, + .perms = p, + }}; + + if (!xs_mkdir(xenstore, 0, path)) { + xen_be_printf(NULL, 0, "xs_mkdir %s: failed\n", path); + return -1; + } + xenstore_cleanup_dir(g_strdup(path)); + + if (!xs_set_permissions(xenstore, 0, path, perms, 2)) { + xen_be_printf(NULL, 0, "xs_set_permissions %s: failed\n", path); + return -1; + } + return 0; +} + int xenstore_write_int(const char *base, const char *node, int ival) { char val[12]; @@ -726,6 +772,12 @@ err: int xen_be_register(const char *type, struct XenDevOps *ops) { + char path[50]; + + snprintf(path, sizeof (path), "device-model/%u/backends/%s", xen_domid, + type); + xenstore_mkdir(path, XS_PERM_READ); + return xenstore_scan(type, xen_domid, ops); } diff --git a/hw/xen/xen_devconfig.c b/hw/xen/xen_devconfig.c index 1f30fe4..b7d290d 100644 --- a/hw/xen/xen_devconfig.c +++ b/hw/xen/xen_devconfig.c @@ -5,54 +5,6 @@ /* ------------------------------------------------------------- */ -struct xs_dirs { - char *xs_dir; - QTAILQ_ENTRY(xs_dirs) list; -}; -static QTAILQ_HEAD(xs_dirs_head, xs_dirs) xs_cleanup = QTAILQ_HEAD_INITIALIZER(xs_cleanup); - -static void xen_config_cleanup_dir(char *dir) -{ - struct xs_dirs *d; - - d = g_malloc(sizeof(*d)); - d->xs_dir = dir; - QTAILQ_INSERT_TAIL(&xs_cleanup, d, list); -} - -void xen_config_cleanup(void) -{ - struct xs_dirs *d; - - QTAILQ_FOREACH(d, &xs_cleanup, list) { - xs_rm(xenstore, 0, d->xs_dir); - } -} - -/* ------------------------------------------------------------- */ - -static int xen_config_dev_mkdir(char *dev, int p) -{ - struct xs_permissions perms[2] = {{ - .id = 0, /* set owner: dom0 */ - },{ - .id = xen_domid, - .perms = p, - }}; - - if (!xs_mkdir(xenstore, 0, dev)) { - xen_be_printf(NULL, 0, "xs_mkdir %s: failed\n", dev); - return -1; - } - xen_config_cleanup_dir(g_strdup(dev)); - - if (!xs_set_permissions(xenstore, 0, dev, perms, 2)) { - xen_be_printf(NULL, 0, "xs_set_permissions %s: failed\n", dev); - return -1; - } - return 0; -} - static int xen_config_dev_dirs(const char *ftype, const char *btype, int vdev, char *fe, char *be, int len) { @@ -66,8 +18,8 @@ static int xen_config_dev_dirs(const char *ftype, const char *btype, int vdev, snprintf(be, len, "%s/backend/%s/%d/%d", dom, btype, xen_domid, vdev); free(dom); - xen_config_dev_mkdir(fe, XS_PERM_READ | XS_PERM_WRITE); - xen_config_dev_mkdir(be, XS_PERM_READ); + xenstore_mkdir(fe, XS_PERM_READ | XS_PERM_WRITE); + xenstore_mkdir(be, XS_PERM_READ); return 0; } diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h index b4b4ff0..1bc5042 100644 --- a/include/hw/xen/xen_backend.h +++ b/include/hw/xen/xen_backend.h @@ -63,6 +63,7 @@ extern const char *xen_protocol; extern DeviceState *xen_sysdev; /* xenstore helper functions */ +int xenstore_mkdir(char *path, int p); int xenstore_write_str(const char *base, const char *node, const char *val); int xenstore_write_int(const char *base, const char *node, int ival); int xenstore_write_int64(const char *base, const char *node, int64_t ival);
Add a Xenstore directory for each supported pv backend. This will allow Xen tools to decide which backend type to use in case there are multiple possibilities. The information is added under /local/domain/<backend-domid>/device-model/<domid>/backends before the "running" state is written to Xenstore. Using a directory for each directory enables us to add parameters for specific backends in the future. In order to reuse the Xenstore directory creation already present in hw/xen/xen_devconfig.c move the related functions to hw/xen/xen_backend.c where they fit better. Signed-off-by: Juergen Gross <jgross@suse.com> --- hw/xen/xen_backend.c | 52 ++++++++++++++++++++++++++++++++++++++++++++ hw/xen/xen_devconfig.c | 52 ++------------------------------------------ include/hw/xen/xen_backend.h | 1 + 3 files changed, 55 insertions(+), 50 deletions(-)