On Thu, Jul 18, 2013 at 08:14:17PM +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 <bjzhang@suse.com> --- 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 | 88 ++++++++++++++++++++++++++++++++++ src/libxl/libxl_conf.h | 3 ++ src/libxl/libxl_driver.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 213 insertions(+)
+static int libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config) { libxl_domain_build_info *b_info = &d_config->b_info; @@ -403,6 +487,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;
If you're going to hardcode def->serials[0], then you should explicitly report an error if 'def->nserials > 1'. Also you should probably report an error if def->nparallels != 0, since you don't support that at all.
+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; + size_t i; + virDomainChrDefPtr chr = NULL; + libxlDomainObjPrivatePtr priv; + char *console = NULL; + size_t 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 (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 (dev_name) { + for (i = 0; !chr && i < vm->def->nconsoles; i++) { + 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++) { + 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++) { + if (STREQ(dev_name, vm->def->parallels[i]->info.alias)) { + chr = vm->def->parallels[i]; + num = i; + } + }
Nothing in the libxl driver appears to ever set the info.alias string names. So this code will crash on a NULL pointer. You should either ensure the alias is set, or use STREQ_NULLABLE to handle NULL pointers.
+ } else { + if (vm->def->nconsoles) + chr = vm->def->consoles[0]; + else if (vm->def->nserials) + chr = vm->def->serials[0]; + }
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 :|