Re: [libvirt] [PATCH V3] add console support in libxl

Hi, Denial thanks your reply. >>> "Daniel P. Berrange" 2013-7-19 下午 23:53 >>> >>> > On Fri, Jul 19, 2013 at 11:48:56PM +0800, 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 >> --- >> Changes since V2: >> 1), forbid parallel configure because libxl do not support it >> 2), only support one serial on libxl driver. >> 3), also remove console code in libxl driver, AFAICS serial is enough for connecting to libxl console. >> changes since V1: >> 1), add virDomainOpenConsoleEnsureACL >> 3), remove virReportOOMErrorFull when virAsprintf fail. >> 4), change size_t for non-nagetive number in libxlDomainOpenConsole >> size_t i; >> size_t num = 0; >> 5), fix for make check >> (1), replace virAsprintf with VIR_STRDUP in two places >> (2), delete space. >> >> src/libxl/libxl_conf.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++ >> src/libxl/libxl_conf.h | 3 ++ >> src/libxl/libxl_driver.c | 87 +++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 190 insertions(+) >> >> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c >> index 4a0fba9..539d537 100644 >> --- a/src/libxl/libxl_conf.c >> +++ b/src/libxl/libxl_conf.c >> @@ -331,6 +331,90 @@ 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 (VIR_STRDUP(*buf, type) < 0) >> + 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) >> + return -1; >> + break; >> + >> + case VIR_DOMAIN_CHR_TYPE_DEV: >> + if (VIR_STRDUP(*buf, def->source.data.file.path) < 0) >> + 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) >> + 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) >> + 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) >> + 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; >> @@ -403,6 +487,22 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain> > + if (def->nserials > 1) { >> + virReportError(VIR_ERR_INTERNAL_ERROR, >> + "%s", _("Only one serial is supported by libxl")); >> + goto error; >> + } >> + if (libxlMakeChrdevStr(def->serials[0], &b_info->u.hvm.serial) < 0) >> + goto error; >> + } >> + >> + if (def->nparallels) { >> + virReportError(VIR_ERR_INTERNAL_ERROR, >> + "%s", _("Parallel is not supported")); >> + 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 358d387..b3a8d50 100644 >> --- a/src/libxl/libxl_driver.c >> +++ b/src/libxl/libxl_driver.c >> @@ -417,6 +417,9 @@ libxlDomainObjPrivateAlloc(void) >> >> libxl_osevent_register_hooks(priv->ctx, &libxl_event_callbacks, priv); >> >> + if (!(priv->devs = virChrdevAlloc())) >> + return NULL; >> + >> return priv; >> } >> >> @@ -429,6 +432,7 @@ libxlDomainObjPrivateDispose(void *obj) >> libxl_evdisable_domain_death(priv->ctx, priv->deathW); >> >> libxl_ctx_free(priv->ctx); >> + virChrdevFree(priv->devs); >> } >> >> static void >> @@ -4493,6 +4497,88 @@ cleanup: >> return ret; >> } >> >> + >> +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; >> + virDomainChrDefPtr chr = NULL; >> + libxlDomainObjPrivatePtr priv; >> + char *console = NULL; >> + >> + 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 (virDomainOpenConsoleEnsureACL(dom->conn, vm->def) < 0) >> + goto cleanup; >> + >> + if (!virDomainObjIsActive(vm)) { >> + virReportError(VIR_ERR_OPERATION_INVALID, >> + "%s", _("domain is not running")); >> + goto cleanup; >> + } >> + >> + priv = vm->privateData; >> + >> + if (vm->def->nserials) >> + chr = vm->def->serials[0]; >> > Now you're ignoring 'dev_name' completely which is definitely not > right. You should be assigning aliases to the serial devices when > starting the guest, so you can match them here. > sorry, maybe i am not get your point. the pty device is get from libxl lib through the libxl_primary_console_get_tty. then pass it to virChrdevOpen. i do not understand why save it in another place.