<stefano.stabellini@eu.citrix.com>
2011-Jun-28 11:34 UTC
[Xen-devel] [PATCH] xen_console: support the new extended xenstore protocol
From: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Since CS 21994 on xen-unstable.hg and CS 466608f3a32e1f9808acdf832a5843af37e5fcec on qemu-xen-unstable.git, few changes have been introduced to the PV console xenstore protocol, as described by the document docs/misc/console.txt under xen-unstable.hg.>From the Qemu point of view, very few modifications are needed tocorrectly support the protocol: read from xenstore the "output" node that tell us what the output of the PV console is going to be. In case the output is a tty, write to xenstore the device name. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- hw/xen.h | 1 + hw/xen_console.c | 16 ++++++++----- xen-all.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 6 deletions(-) diff --git a/hw/xen.h b/hw/xen.h index d435ca0..dad0ca0 100644 --- a/hw/xen.h +++ b/hw/xen.h @@ -50,6 +50,7 @@ qemu_irq *xen_interrupt_controller_init(void); int xen_init(void); int xen_hvm_init(void); void xen_vcpu_init(void); +void xenstore_store_pv_console_info(int i, struct CharDriverState *chr); #if defined(NEED_CPU_H) && !defined(CONFIG_USER_ONLY) void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size); diff --git a/hw/xen_console.c b/hw/xen_console.c index 519d5f5..e81afcd 100644 --- a/hw/xen_console.c +++ b/hw/xen_console.c @@ -179,8 +179,9 @@ static void xencons_send(struct XenConsole *con) static int con_init(struct XenDevice *xendev) { struct XenConsole *con = container_of(xendev, struct XenConsole, xendev); - char *type, *dom; + char *type, *dom, label[32]; int ret = 0; + const char *output; /* setup */ dom = xs_get_domain_path(xenstore, con->xendev.dom); @@ -194,11 +195,14 @@ static int con_init(struct XenDevice *xendev) goto out; } - if (!serial_hds[con->xendev.dev]) - xen_be_printf(xendev, 1, "WARNING: serial line %d not configured\n", - con->xendev.dev); - else - con->chr = serial_hds[con->xendev.dev]; + output = xenstore_read_str(con->console, "output"); + /* output is a pty by default */ + if (output == NULL) { + output = "pty"; + } + snprintf(label, sizeof(label), "xencons%d", con->xendev.dev); + con->chr = qemu_chr_open(label, output, NULL); + xenstore_store_pv_console_info(con->xendev.dev, con->chr); out: qemu_free(type); diff --git a/xen-all.c b/xen-all.c index 6099bff..ee51324 100644 --- a/xen-all.c +++ b/xen-all.c @@ -737,6 +737,68 @@ static void cpu_handle_ioreq(void *opaque) } } +static int store_dev_info(int domid, CharDriverState *cs, const char *string) +{ + struct xs_handle *xs; + char *path; + char *newpath; + char *pts; + + /* + * Only continue if we''re talking to a pty + */ + if (strncmp(cs->filename, "pty:", 4)) { + return 0; + } + pts = cs->filename + 4; + + /* We now have everything we need to set the xenstore entry. */ + xs = xs_daemon_open(); + if (xs == NULL) { + fprintf(stderr, "Could not contact XenStore\n"); + return -1; + } + + path = xs_get_domain_path(xs, domid); + if (path == NULL) { + fprintf(stderr, "xs_get_domain_path() error\n"); + return -1; + } + newpath = realloc(path, (strlen(path) + strlen(string) + + strlen("/tty") + 1)); + if (newpath == NULL) { + free(path); /* realloc errors leave old block */ + fprintf(stderr, "realloc error\n"); + return -1; + } + path = newpath; + + strcat(path, string); + strcat(path, "/tty"); + if (!xs_write(xs, XBT_NULL, path, pts, strlen(pts))) { + fprintf(stderr, "xs_write for ''%s'' fail", string); + return -1; + } + + free(path); + xs_daemon_close(xs); + + return 0; +} + +void xenstore_store_pv_console_info(int i, CharDriverState *chr) +{ + char buf[32]; + + if (i == 0) { + snprintf(buf, sizeof(buf), "/console"); + store_dev_info(xen_domid, chr, buf); + } else { + snprintf(buf, sizeof(buf), "/device/console/%d", i); + store_dev_info(xen_domid, chr, buf); + } +} + static void xenstore_record_dm_state(XenIOState *s, const char *state) { char path[50]; -- 1.7.2.3 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Peter Maydell
2011-Jun-28 12:03 UTC
[Xen-devel] Re: [Qemu-devel] [PATCH] xen_console: support the new extended xenstore protocol
On 28 June 2011 12:34, <stefano.stabellini@eu.citrix.com> wrote:> + path = xs_get_domain_path(xs, domid); > + if (path == NULL) { > + fprintf(stderr, "xs_get_domain_path() error\n"); > + return -1; > + }Don''t we need to call xs_daemon_close() on these error-exit paths?> + if (!xs_write(xs, XBT_NULL, path, pts, strlen(pts))) { > + fprintf(stderr, "xs_write for ''%s'' fail", string); > + return -1; > + }...and this one''s leaking the path string too.> +void xenstore_store_pv_console_info(int i, CharDriverState *chr) > +{ > + char buf[32]; > + > + if (i == 0) { > + snprintf(buf, sizeof(buf), "/console"); > + store_dev_info(xen_domid, chr, buf);Am I missing something, or could you just pass "/console" straight to store_dev_info() without bothering to copy it into buf[] here? -- PMM _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2011-Jun-28 14:21 UTC
[Xen-devel] Re: [Qemu-devel] [PATCH] xen_console: support the new extended xenstore protocol
On Tue, 28 Jun 2011, Peter Maydell wrote:> On 28 June 2011 12:34, <stefano.stabellini@eu.citrix.com> wrote: > > + path = xs_get_domain_path(xs, domid); > > + if (path == NULL) { > > + fprintf(stderr, "xs_get_domain_path() error\n"); > > + return -1; > > + } > > Don''t we need to call xs_daemon_close() on these error-exit paths? > > > + if (!xs_write(xs, XBT_NULL, path, pts, strlen(pts))) { > > + fprintf(stderr, "xs_write for ''%s'' fail", string); > > + return -1; > > + } > > ...and this one''s leaking the path string too. > > > +void xenstore_store_pv_console_info(int i, CharDriverState *chr) > > +{ > > + char buf[32]; > > + > > + if (i == 0) { > > + snprintf(buf, sizeof(buf), "/console"); > > + store_dev_info(xen_domid, chr, buf); > > Am I missing something, or could you just pass "/console" straight > to store_dev_info() without bothering to copy it into buf[] here?All very good points as usual. I''ll send another patch with the error paths fixed and the useless snprintf removed. --8323329-29344616-1309270898=:12963 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --8323329-29344616-1309270898=:12963--