[libvirt] [PATCH] add console support in libxl

Jim Fehlig jfehlig at suse.com
Fri Jul 12 15:44:03 UTC 2013


On 07/04/2013 05:58 AM, Bamvor Jian Zhang wrote:
> this patch introduce the console api in libxl driver for both pv and
> hvm guest.  and import and update the libxlMakeChrdevStr function
> which was deleted in commit dfa1e1dd.
>
> Signed-off-by: Bamvor Jian Zhang <bjzhang at suse.com>
> ---
>   src/libxl/libxl_conf.c   |  97 ++++++++++++++++++++++++++++++++++++++
>   src/libxl/libxl_conf.h   |   3 ++
>   src/libxl/libxl_driver.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 219 insertions(+)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index e170357..08095bc 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -332,6 +332,99 @@ error:
>   }
>   
>   static int
> +libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf)
> +{
> +    const char *type = virDomainChrTypeToString(def->source.type);
> +    int ret;
> +
> +    if (!type) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s", _("unexpected chr device type"));
> +        return -1;
> +    }
> +
> +    switch (def->source.type) {
> +        case VIR_DOMAIN_CHR_TYPE_NULL:
> +        case VIR_DOMAIN_CHR_TYPE_STDIO:
> +        case VIR_DOMAIN_CHR_TYPE_VC:
> +        case VIR_DOMAIN_CHR_TYPE_PTY:
> +            if (virAsprintf(buf, "%s", type) < 0) {
> +                virReportOOMError();

This will need rebased now that Michal's "Introduce OOM reporting to 
virAsprintf" patchset is committed.

https://www.redhat.com/archives/libvir-list/2013-July/msg00506.html

> +                return -1;
> +            }
> +            break;
> +
> +        case VIR_DOMAIN_CHR_TYPE_FILE:
> +        case VIR_DOMAIN_CHR_TYPE_PIPE:
> +            if (virAsprintf(buf, "%s:%s", type,
> +                            def->source.data.file.path) < 0) {
> +                virReportOOMError();
> +                return -1;
> +            }
> +            break;
> +
> +        case VIR_DOMAIN_CHR_TYPE_DEV:
> +            if (virAsprintf(buf, "%s", def->source.data.file.path) < 0) {
> +                virReportOOMError();
> +                return -1;
> +            }
> +            break;
> +        case VIR_DOMAIN_CHR_TYPE_UDP: {
> +            const char *connectHost = def->source.data.udp.connectHost;
> +            const char *bindHost = def->source.data.udp.bindHost;
> +            const char *bindService  = def->source.data.udp.bindService;
> +
> +            if (connectHost == NULL)
> +                connectHost = "";
> +            if (bindHost == NULL)
> +                bindHost = "";
> +            if (bindService == NULL)
> +                bindService = "0";
> +
> +            ret = virAsprintf(buf, "udp:%s:%s@%s:%s",
> +                              connectHost,
> +                              def->source.data.udp.connectService,
> +                              bindHost,
> +                              bindService);
> +            if ( ret < 0) {

Extra space between '(' and 'ret', caught by 'make syntax-check'.

> +                virReportOOMError();
> +                return -1;
> +            }
> +            break;
> +        }
> +        case VIR_DOMAIN_CHR_TYPE_TCP:
> +            if (def->source.data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET) {
> +                ret = virAsprintf(buf, "telnet:%s:%s%s",
> +                                  def->source.data.tcp.host,
> +                                  def->source.data.tcp.service,
> +                                  def->source.data.tcp.listen ? ",server,nowait" : "");
> +            } else {
> +                ret = virAsprintf(buf, "tcp:%s:%s%s",
> +                                  def->source.data.tcp.host,
> +                                  def->source.data.tcp.service,
> +                                  def->source.data.tcp.listen ? ",server,nowait" : "");
> +            }
> +            if ( ret < 0) {

Extra space here too.

> +                virReportOOMError();
> +                return -1;
> +            }
> +            break;
> +
> +        case VIR_DOMAIN_CHR_TYPE_UNIX:
> +            ret = virAsprintf(buf, "unix:%s%s",
> +                              def->source.data.nix.path,
> +                              def->source.data.nix.listen ? ",server,nowait" : "");
> +            if ( ret < 0) {

And here.

BTW, do all of these types work with Xen?  I've only tested with type 'pty', 
which works fine with both pv and hvm guests.

> +                virReportOOMError();
> +                return -1;
> +            }
> +            break;
> +    }
> +
> +    return 0;
> +}
> +
> +static int
>   libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config)
>   {
>       libxl_domain_build_info *b_info = &d_config->b_info;
> @@ -404,6 +497,10 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config)
>           if (VIR_STRDUP(b_info->u.hvm.boot, bootorder) < 0)
>               goto error;
>   
> +        if (def->nserials &&
> +            (libxlMakeChrdevStr(def->serials[0], &b_info->u.hvm.serial) < 0))
> +            goto error;
> +
>           /*
>            * The following comment and calculation were taken directly from
>            * libxenlight's internal function libxl_get_required_shadow_memory():
> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> index 2b4a281..861d689 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -34,6 +34,7 @@
>   # include "configmake.h"
>   # include "virportallocator.h"
>   # include "virobject.h"
> +# include "virchrdev.h"
>   
>   
>   # define LIBXL_VNC_PORT_MIN  5900
> @@ -94,6 +95,8 @@ struct _libxlDomainObjPrivate {
>   
>       /* per domain libxl ctx */
>       libxl_ctx *ctx;
> +    /* console */
> +    virChrdevsPtr devs;
>       libxl_evgen_domain_death *deathW;
>   
>       /* list of libxl timeout registrations */
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 1bae3d6..b8c6832 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -420,6 +420,9 @@ libxlDomainObjPrivateAlloc(void)
>   
>       libxl_osevent_register_hooks(priv->ctx, &libxl_event_callbacks, priv);
>   
> +    if (!(priv->devs = virChrdevAlloc()))
> +        return NULL;
> +
>       return priv;
>   }
>   
> @@ -432,6 +435,7 @@ libxlDomainObjPrivateDispose(void *obj)
>           libxl_evdisable_domain_death(priv->ctx, priv->deathW);
>   
>       libxl_ctx_free(priv->ctx);
> +    virChrdevFree(priv->devs);
>   }
>   
>   static void
> @@ -4518,6 +4522,120 @@ libxlDomainSetSchedulerParameters(virDomainPtr dom, virTypedParameterPtr params,
>       return libxlDomainSetSchedulerParametersFlags(dom, params, nparams, 0);
>   }
>   
> +
> +static int
> +libxlDomainOpenConsole(virDomainPtr dom,
> +                       const char *dev_name,
> +                       virStreamPtr st,
> +                       unsigned int flags)
> +{
> +    libxlDriverPrivatePtr driver = dom->conn->privateData;
> +    virDomainObjPtr vm = NULL;
> +    int ret = -1;
> +    int i;

Needs to be size_t following Daniel's patchset to "Santize iterator variable 
names & data types"

https://www.redhat.com/archives/libvir-list/2013-July/msg00397.html

> +    virDomainChrDefPtr chr = NULL;
> +    libxlDomainObjPrivatePtr priv;
> +    char *console = NULL;
> +    int num = 0;
> +    libxl_console_type type;
> +
> +    virCheckFlags(VIR_DOMAIN_CONSOLE_SAFE |
> +                  VIR_DOMAIN_CONSOLE_FORCE, -1);
> +
> +    libxlDriverLock(driver);
> +    vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> +    libxlDriverUnlock(driver);
> +    if (!vm) {
> +        char uuidstr[VIR_UUID_STRING_BUFLEN];
> +        virUUIDFormat(dom->uuid, uuidstr);
> +        virReportError(VIR_ERR_NO_DOMAIN,
> +                       _("No domain with matching uuid '%s'"), uuidstr);
> +        goto cleanup;
> +    }
> +
> +    if (!virDomainObjIsActive(vm)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       "%s", _("domain is not running"));
> +        goto cleanup;
> +    }
> +
> +    priv = vm->privateData;
> +
> +    if (dev_name) {
> +        for (i = 0 ; !chr && i < vm->def->nconsoles ; i++) {

Spaces before the semicolons causes 'make syntax-check' failure.

> +            if (vm->def->consoles[i]->info.alias &&
> +                STREQ(dev_name, vm->def->consoles[i]->info.alias)) {
> +                chr = vm->def->consoles[i];
> +                num = i;
> +            }
> +        }
> +        for (i = 0 ; !chr && i < vm->def->nserials ; i++) {

Same comment here about spaces before semicolon.

> +            if (STREQ(dev_name, vm->def->serials[i]->info.alias)) {
> +                chr = vm->def->serials[i];
> +                num = i;
> +            }
> +        }
> +        for (i = 0 ; !chr && i < vm->def->nparallels ; i++) {

And here.

> +            if (STREQ(dev_name, vm->def->parallels[i]->info.alias)) {
> +                chr = vm->def->parallels[i];
> +                num = i;
> +            }
> +        }
> +    } else {
> +        if (vm->def->nconsoles)
> +            chr = vm->def->consoles[0];
> +        else if (vm->def->nserials)
> +            chr = vm->def->serials[0];
> +    }
> +
> +    if (!chr) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("cannot find character device %s"),
> +                       NULLSTR(dev_name));
> +        goto cleanup;
> +    }
> +
> +    if (chr->source.type != VIR_DOMAIN_CHR_TYPE_PTY) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("character device %s is not using a PTY"),
> +                       NULLSTR(dev_name));
> +        goto cleanup;
> +    }
> +
> +    if (dev_name) {
> +        if (STREQ(vm->def->os.type, "hvm"))
> +            type = LIBXL_CONSOLE_TYPE_SERIAL;
> +        else
> +            type = LIBXL_CONSOLE_TYPE_PV;
> +        ret = libxl_console_get_tty(priv->ctx, vm->def->id, num, type, &console);
> +    } else {
> +        ret = libxl_primary_console_get_tty(priv->ctx, vm->def->id, &console);
> +    }
> +    if ( ret )

Extra spaces here causing 'make syntax-check' failure as well.

Regards,
Jim

> +        goto cleanup;
> +
> +    if (VIR_STRDUP(chr->source.data.file.path, console) < 0)
> +        goto cleanup;
> +
> +    /* handle mutually exclusive access to console devices */
> +    ret = virChrdevOpen(priv->devs,
> +                         &chr->source,
> +                         st,
> +                         (flags & VIR_DOMAIN_CONSOLE_FORCE) != 0);
> +
> +    if (ret == 1) {
> +        virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +                       _("Active console session exists for this domain"));
> +        ret = -1;
> +    }
> +
> +cleanup:
> +    VIR_FREE(console);
> +    if (vm)
> +        virObjectUnlock(vm);
> +    return ret;
> +}
> +
>   static int
>   libxlDomainIsActive(virDomainPtr dom)
>   {
> @@ -4739,6 +4857,7 @@ static virDriver libxlDriver = {
>       .domainManagedSave = libxlDomainManagedSave, /* 0.9.2 */
>       .domainHasManagedSaveImage = libxlDomainHasManagedSaveImage, /* 0.9.2 */
>       .domainManagedSaveRemove = libxlDomainManagedSaveRemove, /* 0.9.2 */
> +    .domainOpenConsole = libxlDomainOpenConsole, /* 1.0.8 */
>       .domainIsActive = libxlDomainIsActive, /* 0.9.0 */
>       .domainIsPersistent = libxlDomainIsPersistent, /* 0.9.0 */
>       .domainIsUpdated = libxlDomainIsUpdated, /* 0.9.0 */




More information about the libvir-list mailing list