Daniel P. Berrange wrote:
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'.
Yes, good point. Currently libxl only supports defining one serial port, so we should enforce that here.
Also you should probably report an error if def->nparallels != 0, since you don't support that at all.
Right. Maybe that should be a separate patch. Poking around a bit, I'm not even sure how to map libvirt's parallel device config to libxl's 'num_ioports' and 'ioports' fields in libxl_domain_build_info struct. xl.cfg(5) contains this info about the 'ioports' setting ioports=[ "IOPORT_RANGE", "IOPORT_RANGE", ... ] Allow guest to access specific legacy I/O ports. Each IOPORT_RANGE is given in hexadecimal and may either a span e.g. "2f8-2ff" (inclusive) or a single I/O port "2f8". It is recommended to use this option only for trusted VMs under administrator control. Regards, Jim