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

[Bamvor Jian Zhang]

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.


thanks


Bamvor
> 
> 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 :|
> 
>