Re: [Qemu-devel] [PATCH v3] xen_console: support the new extended xenstore protocol

[Alexander Graf <agraf@suse.de>]

On 06/29/2011 01:12 PM, stefano.stabellini@eu.citrix.com wrote:
> 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 to
> correctly 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.
>
> Changes in v2:
>
> - fix error paths: free malloc'ed strings and close the xenstore
> connection before returning;
>
> - remove useless snprintf in xenstore_store_pv_console_info if i == 0.
>
> Changes in v3:
>
> - replace xs_daemon_open/xs_daemon_close with xs_open/xs_close.
>
> Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
> ---
>   hw/xen.h         |    1 +
>   hw/xen_console.c |   16 +++++++++-----
>   xen-all.c        |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 71 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";

Not sure I understand. Why would a pty be the default? stdio makes a lot 
more sense, no?

> +    }
> +    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..3fd04ef 100644
> --- a/xen-all.c
> +++ b/xen-all.c
> @@ -737,6 +737,66 @@ static void cpu_handle_ioreq(void *opaque)
>       }
>   }
>
> +static int store_dev_info(int domid, CharDriverState *cs, const char *string)
> +{
> +    struct xs_handle *xs = NULL;
> +    char *path = NULL;
> +    char *newpath = NULL;
> +    char *pts = NULL;
> +    int ret = -1;
> +
> +    /* Only continue if we're talking to a pty. */
> +    if (strncmp(cs->filename, "pty:", 4)) {

What's wrong with !pty?


Alex