[libvirt] [PATCH] add console support in libxl
Bamvor Jian Zhang
bjzhang at suse.com
Wed Jul 17 03:27:38 UTC 2013
Hi, Jim
thanks your reply. one comment below.
>>>已写入 "Jim Fehlig <jfehlig at suse.com>"> 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.
i only test the pty type too. lots of type is introduced in 2b84e445(Add libxenlight driver), i add the missing type compare with qemu driver. maybe it will be used in future. for now, only pty is needed for my console patch.
thanks
Bamvor
> > + 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