Hi, Jim thanks your reply. one comment below. >>>已写入 "Jim Fehlig <jfehlig@xxxxxxxx>"> 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@xxxxxxxx> > > --- > > 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 */ > > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list