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

>> "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.

Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|