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@xxxxxxxx>
> ---
> 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list