| Message ID | 1379533858-7645-1-git-send-email-roger.pau@citrix.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Sep 18, 2013 at 09:50:58PM +0200, Roger Pau Monne wrote: > Qemu has several hardcoded xenstore paths that are only valid on Dom0. > Attempts to launch a Qemu instance (to act as a userspace backend for > PV disks) will fail because Qemu is not able to access those paths > when running on a domain different than Dom0. > > Instead make the xenstore paths relative to the domain where Qemu is > actually running. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Cc: xen-devel@lists.xenproject.org > Cc: Anthony PERARD <anthony.perard@citrix.com> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> This look fine. One issue with the patch: the file xen_backend.c have been moved to hw/xen/xen_backend.c. I've also tryied it in a stubdomain, and it does not boot anymore because the qemu in the stubdom can not read the state. I have tried again without the change in xen-all.c, and the stubdom does not complain anymore. So in the change in xenstore_record_dm_state() needed as well? > --- > hw/xen_backend.c | 19 ++++++------------- > xen-all.c | 2 +- > 2 files changed, 7 insertions(+), 14 deletions(-) > > diff --git a/hw/xen_backend.c b/hw/xen_backend.c > index 008cdb3..e220606 100644 > --- a/hw/xen_backend.c > +++ b/hw/xen_backend.c > @@ -205,7 +205,6 @@ static struct XenDevice *xen_be_get_xendev(const char *type, int dom, int dev, > struct XenDevOps *ops) > { > struct XenDevice *xendev; > - char *dom0; > > xendev = xen_be_find_xendev(type, dom, dev); > if (xendev) { > @@ -219,12 +218,10 @@ static struct XenDevice *xen_be_get_xendev(const char *type, int dom, int dev, > xendev->dev = dev; > xendev->ops = ops; > > - dom0 = xs_get_domain_path(xenstore, 0); > - snprintf(xendev->be, sizeof(xendev->be), "%s/backend/%s/%d/%d", > - dom0, xendev->type, xendev->dom, xendev->dev); > + snprintf(xendev->be, sizeof(xendev->be), "backend/%s/%d/%d", > + xendev->type, xendev->dom, xendev->dev); > snprintf(xendev->name, sizeof(xendev->name), "%s-%d", > xendev->type, xendev->dev); > - free(dom0); > > xendev->debug = debug; > xendev->local_port = -1; > @@ -570,14 +567,12 @@ static int xenstore_scan(const char *type, int dom, struct XenDevOps *ops) > { > struct XenDevice *xendev; > char path[XEN_BUFSIZE], token[XEN_BUFSIZE]; > - char **dev = NULL, *dom0; > + char **dev = NULL; > unsigned int cdev, j; > > /* setup watch */ > - dom0 = xs_get_domain_path(xenstore, 0); > snprintf(token, sizeof(token), "be:%p:%d:%p", type, dom, ops); > - snprintf(path, sizeof(path), "%s/backend/%s/%d", dom0, type, dom); > - free(dom0); > + snprintf(path, sizeof(path), "backend/%s/%d", type, dom); > if (!xs_watch(xenstore, path, token)) { > xen_be_printf(NULL, 0, "xen be: watching backend path (%s) failed\n", path); > return -1; > @@ -603,12 +598,10 @@ static void xenstore_update_be(char *watch, char *type, int dom, > struct XenDevOps *ops) > { > struct XenDevice *xendev; > - char path[XEN_BUFSIZE], *dom0, *bepath; > + char path[XEN_BUFSIZE], *bepath; > unsigned int len, dev; > > - dom0 = xs_get_domain_path(xenstore, 0); > - len = snprintf(path, sizeof(path), "%s/backend/%s/%d", dom0, type, dom); > - free(dom0); > + len = snprintf(path, sizeof(path), "backend/%s/%d", type, dom); > if (strncmp(path, watch, len) != 0) { > return; > } > diff --git a/xen-all.c b/xen-all.c > index 15be8ed..99666f9 100644 > --- a/xen-all.c > +++ b/xen-all.c > @@ -967,7 +967,7 @@ static void xenstore_record_dm_state(struct xs_handle *xs, const char *state) > exit(1); > } > > - snprintf(path, sizeof (path), "/local/domain/0/device-model/%u/state", xen_domid); > + snprintf(path, sizeof (path), "device-model/%u/state", xen_domid); > if (!xs_write(xs, XBT_NULL, path, state, strlen(state))) { > fprintf(stderr, "error recording dm state\n"); > exit(1); > -- > 1.7.7.5 (Apple Git-26) >
On 26/09/13 18:46, Anthony PERARD wrote: > On Wed, Sep 18, 2013 at 09:50:58PM +0200, Roger Pau Monne wrote: >> Qemu has several hardcoded xenstore paths that are only valid on Dom0. >> Attempts to launch a Qemu instance (to act as a userspace backend for >> PV disks) will fail because Qemu is not able to access those paths >> when running on a domain different than Dom0. >> >> Instead make the xenstore paths relative to the domain where Qemu is >> actually running. >> >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >> Cc: xen-devel@lists.xenproject.org >> Cc: Anthony PERARD <anthony.perard@citrix.com> >> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > This look fine. One issue with the patch: the file xen_backend.c have > been moved to hw/xen/xen_backend.c. Thanks, this is based on the stable Qemu version in Xen tree, I should have done the change on top of the main qemu.git repo. > I've also tryied it in a stubdomain, and it does not boot anymore > because the qemu in the stubdom can not read the state. I have tried > again without the change in xen-all.c, and the stubdom does not complain > anymore. So in the change in xenstore_record_dm_state() needed as well? Yes, if we run a Qemu instance inside a driver domain it wouldn't make much sense IMHO to write the state of that Qemu instance on a xenstore path that belongs to the Dom0, and also we would need to give the driver domain permissions to write on a xenstore path that's inside the Dom0 xenstore path, which doesn't seem like a good idea. To make Qemu work on a domain different than Dom0 you will also need the following patch from my driver domain series: http://marc.info/?l=xen-devel&m=137993233817018 If not the guest is unable to create the device-model/<domid>/state xenstore entry. For stubdomains would it be really hard to change the Dom0 to check for /local/domain/<stubdom_id>/device-model/<domid>/state instead of /local/domain/0/device-model/<domid>/state?
On Thu, Sep 26, 2013 at 07:20:31PM +0200, Roger Pau Monné wrote: > On 26/09/13 18:46, Anthony PERARD wrote: > > On Wed, Sep 18, 2013 at 09:50:58PM +0200, Roger Pau Monne wrote: > >> Qemu has several hardcoded xenstore paths that are only valid on Dom0. > >> Attempts to launch a Qemu instance (to act as a userspace backend for > >> PV disks) will fail because Qemu is not able to access those paths > >> when running on a domain different than Dom0. > >> > >> Instead make the xenstore paths relative to the domain where Qemu is > >> actually running. > >> > >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > >> Cc: xen-devel@lists.xenproject.org > >> Cc: Anthony PERARD <anthony.perard@citrix.com> > >> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > This look fine. One issue with the patch: the file xen_backend.c have > > been moved to hw/xen/xen_backend.c. > > Thanks, this is based on the stable Qemu version in Xen tree, I should > have done the change on top of the main qemu.git repo. > > > I've also tryied it in a stubdomain, and it does not boot anymore > > because the qemu in the stubdom can not read the state. I have tried > > again without the change in xen-all.c, and the stubdom does not complain > > anymore. So in the change in xenstore_record_dm_state() needed as well? > > Yes, if we run a Qemu instance inside a driver domain it wouldn't make > much sense IMHO to write the state of that Qemu instance on a xenstore > path that belongs to the Dom0, and also we would need to give the driver > domain permissions to write on a xenstore path that's inside the Dom0 > xenstore path, which doesn't seem like a good idea. > > To make Qemu work on a domain different than Dom0 you will also need the > following patch from my driver domain series: > > http://marc.info/?l=xen-devel&m=137993233817018 > > If not the guest is unable to create the device-model/<domid>/state > xenstore entry. For stubdomains would it be really hard to change the > Dom0 to check for /local/domain/<stubdom_id>/device-model/<domid>/state > instead of /local/domain/0/device-model/<domid>/state? I have tried with the patch applied to libxl, and stubdom work fine with the changes to the xenstore paths. So, once the xen_backend.c file is in the new path, you can add my: Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
diff --git a/hw/xen_backend.c b/hw/xen_backend.c index 008cdb3..e220606 100644 --- a/hw/xen_backend.c +++ b/hw/xen_backend.c @@ -205,7 +205,6 @@ static struct XenDevice *xen_be_get_xendev(const char *type, int dom, int dev, struct XenDevOps *ops) { struct XenDevice *xendev; - char *dom0; xendev = xen_be_find_xendev(type, dom, dev); if (xendev) { @@ -219,12 +218,10 @@ static struct XenDevice *xen_be_get_xendev(const char *type, int dom, int dev, xendev->dev = dev; xendev->ops = ops; - dom0 = xs_get_domain_path(xenstore, 0); - snprintf(xendev->be, sizeof(xendev->be), "%s/backend/%s/%d/%d", - dom0, xendev->type, xendev->dom, xendev->dev); + snprintf(xendev->be, sizeof(xendev->be), "backend/%s/%d/%d", + xendev->type, xendev->dom, xendev->dev); snprintf(xendev->name, sizeof(xendev->name), "%s-%d", xendev->type, xendev->dev); - free(dom0); xendev->debug = debug; xendev->local_port = -1; @@ -570,14 +567,12 @@ static int xenstore_scan(const char *type, int dom, struct XenDevOps *ops) { struct XenDevice *xendev; char path[XEN_BUFSIZE], token[XEN_BUFSIZE]; - char **dev = NULL, *dom0; + char **dev = NULL; unsigned int cdev, j; /* setup watch */ - dom0 = xs_get_domain_path(xenstore, 0); snprintf(token, sizeof(token), "be:%p:%d:%p", type, dom, ops); - snprintf(path, sizeof(path), "%s/backend/%s/%d", dom0, type, dom); - free(dom0); + snprintf(path, sizeof(path), "backend/%s/%d", type, dom); if (!xs_watch(xenstore, path, token)) { xen_be_printf(NULL, 0, "xen be: watching backend path (%s) failed\n", path); return -1; @@ -603,12 +598,10 @@ static void xenstore_update_be(char *watch, char *type, int dom, struct XenDevOps *ops) { struct XenDevice *xendev; - char path[XEN_BUFSIZE], *dom0, *bepath; + char path[XEN_BUFSIZE], *bepath; unsigned int len, dev; - dom0 = xs_get_domain_path(xenstore, 0); - len = snprintf(path, sizeof(path), "%s/backend/%s/%d", dom0, type, dom); - free(dom0); + len = snprintf(path, sizeof(path), "backend/%s/%d", type, dom); if (strncmp(path, watch, len) != 0) { return; } diff --git a/xen-all.c b/xen-all.c index 15be8ed..99666f9 100644 --- a/xen-all.c +++ b/xen-all.c @@ -967,7 +967,7 @@ static void xenstore_record_dm_state(struct xs_handle *xs, const char *state) exit(1); } - snprintf(path, sizeof (path), "/local/domain/0/device-model/%u/state", xen_domid); + snprintf(path, sizeof (path), "device-model/%u/state", xen_domid); if (!xs_write(xs, XBT_NULL, path, state, strlen(state))) { fprintf(stderr, "error recording dm state\n"); exit(1);
Qemu has several hardcoded xenstore paths that are only valid on Dom0. Attempts to launch a Qemu instance (to act as a userspace backend for PV disks) will fail because Qemu is not able to access those paths when running on a domain different than Dom0. Instead make the xenstore paths relative to the domain where Qemu is actually running. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Cc: xen-devel@lists.xenproject.org Cc: Anthony PERARD <anthony.perard@citrix.com> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- hw/xen_backend.c | 19 ++++++------------- xen-all.c | 2 +- 2 files changed, 7 insertions(+), 14 deletions(-)