Re: [Xen-devel] [PATCH v3 2/3] xen: write information about supported backends
On Wed, May 11, 2016 at 10:36:28AM +0200, Juergen Gross wrote:
> On 10/05/16 18:21, Anthony PERARD wrote:
> > On Fri, May 06, 2016 at 11:42:45AM +0200, Juergen Gross wrote:
> >> -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.
I guess the function looks generic enough. So I'm now fine with moving it
to xen_backend.c.
--
Anthony PERARD
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/3] xen: write information about supported backends
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//device-model//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
>> ---
>> 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
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/3] xen: write information about supported backends
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//device-model//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
> ---
> 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 do
[Xen-devel] [PATCH v3 2/3] xen: write information about supported backends
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//device-model//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
---
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);
+
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(con
