| Message ID | 1462527766-26462-3-git-send-email-jgross@suse.com |
|---|---|
| State | New |
| Headers | show |
On Fri, May 06, 2016 at 11:42:45AM +0200, 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 backend enables us to add parameters for specific backends > in the future. > > This interface is documented in the Xen source repository in the file > docs/misc/qemu-backends.txt > > 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> > --- > V3: Added .backend_register function to XenDevOps in order to have a > way to let the backend decide whether all prerequisites are met > for support. > > V2: update commit message as requested by Stefano > --- > hw/xen/xen_backend.c | 60 ++++++++++++++++++++++++++++++++++++++++++++ > hw/xen/xen_devconfig.c | 52 ++------------------------------------ > include/hw/xen/xen_backend.h | 2 ++ > 3 files changed, 64 insertions(+), 50 deletions(-) > > diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c > index 60575ad..6d8b3a5 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,20 @@ err: > > int xen_be_register(const char *type, struct XenDevOps *ops) > { > + char path[50]; > + int rc; > + > + if (ops->backend_register) { > + rc = ops->backend_register(); > + if (rc) { > + return rc; > + } > + } > + > + snprintf(path, sizeof (path), "device-model/%u/backends/%s", xen_domid, > + type); > + xenstore_mkdir(path, XS_PERM_READ); Do you actually need the guest to be able to read those paths? > + > 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, > - }}; > - The function looks like it as been tailored to mkdir for config of backends. So it does not seems out of place. > - 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..63364f7 100644 > --- a/include/hw/xen/xen_backend.h > +++ b/include/hw/xen/xen_backend.h > @@ -28,6 +28,7 @@ struct XenDevOps { > int (*free)(struct XenDevice *xendev); > void (*backend_changed)(struct XenDevice *xendev, const char *node); > void (*frontend_changed)(struct XenDevice *xendev, const char *node); > + int (*backend_register)(void); > }; > > struct XenDevice { > @@ -63,6 +64,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.6 >
On 10/05/16 18:21, Anthony PERARD wrote: > On Fri, May 06, 2016 at 11:42:45AM +0200, 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 backend enables us to add parameters for specific backends >> in the future. >> >> This interface is documented in the Xen source repository in the file >> docs/misc/qemu-backends.txt >> >> 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> >> --- >> V3: Added .backend_register function to XenDevOps in order to have a >> way to let the backend decide whether all prerequisites are met >> for support. >> >> V2: update commit message as requested by Stefano >> --- >> hw/xen/xen_backend.c | 60 ++++++++++++++++++++++++++++++++++++++++++++ >> hw/xen/xen_devconfig.c | 52 ++------------------------------------ >> include/hw/xen/xen_backend.h | 2 ++ >> 3 files changed, 64 insertions(+), 50 deletions(-) >> >> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c >> index 60575ad..6d8b3a5 100644 >> --- a/hw/xen/xen_backend.c >> +++ b/hw/xen/xen_backend.c >> @@ -726,6 +772,20 @@ err: >> >> int xen_be_register(const char *type, struct XenDevOps *ops) >> { >> + char path[50]; >> + int rc; >> + >> + if (ops->backend_register) { >> + rc = ops->backend_register(); >> + if (rc) { >> + return rc; >> + } >> + } >> + >> + snprintf(path, sizeof (path), "device-model/%u/backends/%s", xen_domid, >> + type); >> + xenstore_mkdir(path, XS_PERM_READ); > > Do you actually need the guest to be able to read those paths? No, you are right. >> 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, >> - }}; >> - > > The function looks like it as been tailored to mkdir for config of > backends. So it does not seems out of place. It has been tailored for config of _devices_ by the backend. I still think it would fit better now into xen_backend.c, OTOH in case you like it better in xen_devconfig.c I won't argue against it. Juergen
diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c index 60575ad..6d8b3a5 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,20 @@ err: int xen_be_register(const char *type, struct XenDevOps *ops) { + char path[50]; + int rc; + + if (ops->backend_register) { + rc = ops->backend_register(); + if (rc) { + return rc; + } + } + + 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..63364f7 100644 --- a/include/hw/xen/xen_backend.h +++ b/include/hw/xen/xen_backend.h @@ -28,6 +28,7 @@ struct XenDevOps { int (*free)(struct XenDevice *xendev); void (*backend_changed)(struct XenDevice *xendev, const char *node); void (*frontend_changed)(struct XenDevice *xendev, const char *node); + int (*backend_register)(void); }; struct XenDevice { @@ -63,6 +64,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 backend enables us to add parameters for specific backends in the future. This interface is documented in the Xen source repository in the file docs/misc/qemu-backends.txt 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> --- V3: Added .backend_register function to XenDevOps in order to have a way to let the backend decide whether all prerequisites are met for support. V2: update commit message as requested by Stefano --- hw/xen/xen_backend.c | 60 ++++++++++++++++++++++++++++++++++++++++++++ hw/xen/xen_devconfig.c | 52 ++------------------------------------ include/hw/xen/xen_backend.h | 2 ++ 3 files changed, 64 insertions(+), 50 deletions(-)